From b779bdb5a023f08b7b8f911f6aed7eb00703faa6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 28 May 2026 21:39:02 -0400 Subject: [PATCH] fix: centralize cron schedule number coercion --- src/cron/normalize.test.ts | 68 ++++++++++++++++++++++++++++++ src/cron/normalize.ts | 12 +++++- src/cron/schedule-identity.test.ts | 23 ++++++++++ src/cron/schedule-identity.ts | 5 +-- src/cron/schedule-number.ts | 5 +++ src/cron/schedule.test.ts | 2 + src/cron/schedule.ts | 18 ++------ 7 files changed, 114 insertions(+), 19 deletions(-) create mode 100644 src/cron/schedule-identity.test.ts create mode 100644 src/cron/schedule-number.ts diff --git a/src/cron/normalize.test.ts b/src/cron/normalize.test.ts index 81a92580caf..45c562fc87c 100644 --- a/src/cron/normalize.test.ts +++ b/src/cron/normalize.test.ts @@ -623,6 +623,74 @@ describe("normalizeCronJobCreate", () => { expect(validateCronAddParams(normalized)).toBe(true); }); + it("normalizes string every schedule numbers for create jobs", () => { + const normalized = normalizeCronJobCreate({ + name: "every-string", + schedule: { + everyMs: "60000", + anchorMs: "123.9", + }, + sessionTarget: "main", + wakeMode: "next-heartbeat", + payload: { + kind: "systemEvent", + text: "hi", + }, + }) as unknown as Record; + + const schedule = normalized.schedule as Record; + expect(schedule).toEqual({ + kind: "every", + everyMs: 60_000, + anchorMs: 123, + }); + expect(validateCronAddParams(normalized)).toBe(true); + }); + + it("normalizes string every schedule numbers for patches", () => { + const normalized = normalizeCronJobPatch({ + schedule: { + kind: "every", + everyMs: "60000", + anchorMs: "123.9", + }, + }) as unknown as Record; + + const schedule = normalized.schedule as Record; + expect(schedule).toEqual({ + kind: "every", + everyMs: 60_000, + anchorMs: 123, + }); + expect(validateCronUpdateParams({ id: "job", patch: normalized })).toBe(true); + }); + + it("keeps invalid every schedule numbers invalid for validation", () => { + const zeroEvery = normalizeCronJobCreate({ + name: "every-zero", + schedule: { + kind: "every", + everyMs: "0", + }, + sessionTarget: "main", + wakeMode: "next-heartbeat", + payload: { + kind: "systemEvent", + text: "hi", + }, + }) as unknown as Record; + expect(validateCronAddParams(zeroEvery)).toBe(false); + + const negativeAnchor = normalizeCronJobPatch({ + schedule: { + kind: "every", + everyMs: "60000", + anchorMs: "-1", + }, + }) as unknown as Record; + expect(validateCronUpdateParams({ id: "job", patch: negativeAnchor })).toBe(false); + }); + it("coerces sessionTarget and wakeMode casing", () => { const normalized = normalizeCronJobCreate({ name: "casing", diff --git a/src/cron/normalize.ts b/src/cron/normalize.ts index 2b7d124ce5c..802f14401b4 100644 --- a/src/cron/normalize.ts +++ b/src/cron/normalize.ts @@ -13,6 +13,7 @@ import { parseOptionalField, } from "./delivery-field-schemas.js"; import { parseAbsoluteTimeMs } from "./parse.js"; +import { coerceFiniteScheduleNumber } from "./schedule-number.js"; import { inferLegacyName } from "./service/normalize.js"; import { assertSafeCronSessionTargetId, @@ -73,6 +74,8 @@ function coerceSchedule(schedule: UnknownRecord) { const exprRaw = normalizeOptionalString(schedule.expr) ?? ""; const legacyCronRaw = normalizeOptionalString(schedule.cron) ?? ""; const normalizedExpr = exprRaw || legacyCronRaw; + const everyMs = coerceFiniteScheduleNumber(schedule.everyMs); + const anchorMs = coerceFiniteScheduleNumber(schedule.anchorMs); const atMsRaw = schedule.atMs; const atRaw = schedule.at; const atString = normalizeOptionalString(atRaw) ?? ""; @@ -94,7 +97,7 @@ function coerceSchedule(schedule: UnknownRecord) { typeof schedule.atMs === "string" ) { next.kind = "at"; - } else if (typeof schedule.everyMs === "number") { + } else if (everyMs !== undefined) { next.kind = "every"; } else if (normalizedExpr) { next.kind = "cron"; @@ -115,6 +118,13 @@ function coerceSchedule(schedule: UnknownRecord) { } else if ("expr" in next) { delete next.expr; } + + if (everyMs !== undefined && everyMs >= 1) { + next.everyMs = Math.floor(everyMs); + } + if (anchorMs !== undefined && anchorMs >= 0) { + next.anchorMs = Math.floor(anchorMs); + } if ("cron" in next) { delete next.cron; } diff --git a/src/cron/schedule-identity.test.ts b/src/cron/schedule-identity.test.ts new file mode 100644 index 00000000000..64b4365ba8d --- /dev/null +++ b/src/cron/schedule-identity.test.ts @@ -0,0 +1,23 @@ +import { describe, expect, it } from "vitest"; +import { cronSchedulingInputsEqual, tryCronScheduleIdentity } from "./schedule-identity.js"; + +describe("tryCronScheduleIdentity", () => { + it("normalizes numeric schedule strings like execution does", () => { + const numeric = tryCronScheduleIdentity({ + enabled: true, + schedule: { kind: "every", everyMs: 60_000, anchorMs: 123 }, + }); + const stringNumeric = tryCronScheduleIdentity({ + enabled: true, + schedule: { kind: "every", everyMs: "60000", anchorMs: "123" }, + }); + + expect(stringNumeric).toBe(numeric); + expect( + cronSchedulingInputsEqual( + { schedule: { kind: "every", everyMs: 60_000, anchorMs: 123 } }, + { schedule: { kind: "every", everyMs: "60000", anchorMs: "123" } }, + ), + ).toBe(true); + }); +}); diff --git a/src/cron/schedule-identity.ts b/src/cron/schedule-identity.ts index bff1f3387d6..634016c11a7 100644 --- a/src/cron/schedule-identity.ts +++ b/src/cron/schedule-identity.ts @@ -1,5 +1,5 @@ -import { asFiniteNumber } from "../shared/number-coercion.js"; import { normalizeOptionalString } from "../shared/string-coerce.js"; +import { coerceFiniteScheduleNumber } from "./schedule-number.js"; import type { CronJob } from "./types.js"; function readString(record: Record, key: string): string | undefined { @@ -7,8 +7,7 @@ function readString(record: Record, key: string): string | unde } function readNumber(record: Record, key: string): number | undefined { - const value = record[key]; - return asFiniteNumber(value); + return coerceFiniteScheduleNumber(record[key]); } function schedulePayloadFromRecord( diff --git a/src/cron/schedule-number.ts b/src/cron/schedule-number.ts new file mode 100644 index 00000000000..29bcadff3e0 --- /dev/null +++ b/src/cron/schedule-number.ts @@ -0,0 +1,5 @@ +import { parseStrictFiniteNumber } from "../shared/number-coercion.js"; + +export function coerceFiniteScheduleNumber(value: unknown): number | undefined { + return parseStrictFiniteNumber(value); +} diff --git a/src/cron/schedule.test.ts b/src/cron/schedule.test.ts index c9ed10eb08c..6823c779763 100644 --- a/src/cron/schedule.test.ts +++ b/src/cron/schedule.test.ts @@ -250,6 +250,8 @@ describe("coerceFiniteScheduleNumber", () => { it("returns undefined for invalid inputs", () => { expect(coerceFiniteScheduleNumber("")).toBeUndefined(); expect(coerceFiniteScheduleNumber("abc")).toBeUndefined(); + expect(coerceFiniteScheduleNumber("60000ms")).toBeUndefined(); + expect(coerceFiniteScheduleNumber("0x10")).toBeUndefined(); expect(coerceFiniteScheduleNumber(Number.NaN)).toBeUndefined(); expect(coerceFiniteScheduleNumber(Infinity)).toBeUndefined(); expect(coerceFiniteScheduleNumber(null)).toBeUndefined(); diff --git a/src/cron/schedule.ts b/src/cron/schedule.ts index 569c354f1d3..4519df78b13 100644 --- a/src/cron/schedule.ts +++ b/src/cron/schedule.ts @@ -1,8 +1,11 @@ import { Cron } from "croner"; import { normalizeOptionalString } from "../shared/string-coerce.js"; import { parseAbsoluteTimeMs } from "./parse.js"; +import { coerceFiniteScheduleNumber } from "./schedule-number.js"; import type { CronSchedule } from "./types.js"; +export { coerceFiniteScheduleNumber } from "./schedule-number.js"; + const CRON_EVAL_CACHE_MAX = 512; const cronEvalCache = new Map(); @@ -50,21 +53,6 @@ function resolveCronFromSchedule(schedule: { return resolveCachedCron(expr, resolveCronTimezone(schedule.tz)); } -export function coerceFiniteScheduleNumber(value: unknown): number | undefined { - if (typeof value === "number") { - return Number.isFinite(value) ? value : undefined; - } - if (typeof value === "string") { - const trimmed = value.trim(); - if (!trimmed) { - return undefined; - } - const parsed = Number(trimmed); - return Number.isFinite(parsed) ? parsed : undefined; - } - return undefined; -} - export function computeNextRunAtMs(schedule: CronSchedule, nowMs: number): number | undefined { if (schedule.kind === "at") { // Handle both canonical `at` (string) and legacy `atMs` (number) fields.