From 736f7bf042af2d0d2c4466fc8281ca1c6d7cabbc Mon Sep 17 00:00:00 2001 From: "paseo-bot[bot]" <266920839+paseo-bot[bot]@users.noreply.github.com> Date: Tue, 3 Nov 2026 00:56:28 +0000 Subject: [PATCH] Reject cron steps that would silently run hourly (#8607) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(schedules): reject ignored numeric cron steps * fix(schedules): preserve saved numeric steps during recovery Reject unsupported numeric steps at create/update while keeping saved schedules executable with their previous singleton cadence. Co-authored-by: 见安 <73627805+Mar-garet@users.noreply.github.com> --------- Co-authored-by: 见安 <73627805+Mar-garet@users.noreply.github.com> --- .../src/schedule/cron-expression.test.ts | 31 ++++++++ .../protocol/src/schedule/cron-expression.ts | 9 +++ packages/server/src/server/schedule/cron.ts | 10 ++- .../src/server/schedule/service.test.ts | 75 +++++++++++++++++++ 4 files changed, 123 insertions(+), 2 deletions(-) diff --git a/packages/protocol/src/schedule/cron-expression.test.ts b/packages/protocol/src/schedule/cron-expression.test.ts index 0db1d68ea1..cb15451700 100644 --- a/packages/protocol/src/schedule/cron-expression.test.ts +++ b/packages/protocol/src/schedule/cron-expression.test.ts @@ -13,6 +13,37 @@ describe("cron expression parser", () => { expect(cron.dayOfWeek.matches(0)).toBe(false); }); + it("rejects a step after a standalone number", () => { + expect(validateCronExpression("0/15 * * * *")).toBe( + "Invalid cron minute step: use * or a range before /", + ); + }); + + it.each([ + ["5/1 * * * *", "minute"], + ["0,5/15 * * * *", "minute"], + ["* 1/2 * * *", "hour"], + ["* * 1/2 * *", "day-of-month"], + ["* * * 1/2 *", "month"], + ["* * * * 1/2", "day-of-week"], + ])("rejects ignored numeric steps in %s", (expression, field) => { + expect(validateCronExpression(expression)).toBe( + `Invalid cron ${field} step: use * or a range before /`, + ); + }); + + it.each([ + { expression: "*/15 * * * *", expected: [0, 15, 30, 45] }, + { expression: "0-59/15 * * * *", expected: [0, 15, 30, 45] }, + { expression: "5-59/15 * * * *", expected: [5, 20, 35, 50] }, + ])("preserves supported steps in $expression", ({ expression, expected }) => { + const cron = parseCronExpression(expression); + const minutes = Array.from({ length: 60 }, (_, minute) => minute); + const matches = minutes.filter((minute) => cron.minute.matches(minute)); + + expect(matches).toEqual(expected); + }); + it("reports invalid expressions with server-facing copy", () => { expect(validateCronExpression("* * *")).toBe("Cron expressions must have 5 fields"); expect(validateCronExpression("*/5/2 * * * *")).toBe("Invalid cron minute step"); diff --git a/packages/protocol/src/schedule/cron-expression.ts b/packages/protocol/src/schedule/cron-expression.ts index 691631d682..cc93652afa 100644 --- a/packages/protocol/src/schedule/cron-expression.ts +++ b/packages/protocol/src/schedule/cron-expression.ts @@ -90,6 +90,9 @@ function parseField(source: string, bounds: CronFieldBounds): CronFieldMatcher { if (!/^\d+$/.test(base)) { throw new Error(`Invalid cron ${bounds.name} value`); } + // COMPAT(cronNumericSteps): added in v0.12.0, remove after 2027-04-17 once + // persisted numeric steps are migrated. Validation rejects new numeric steps; + // existing schedules retain their historical singleton match. const value = Number.parseInt(base, 10); if (!Number.isInteger(value) || value < bounds.min || value > bounds.max) { throw new Error(`Invalid cron ${bounds.name} value`); @@ -122,6 +125,12 @@ export function parseCronExpression(expression: string): ParsedCronExpression { export function validateCronExpression(expression: string): string | null { try { parseCronExpression(expression); + const fields = expression.trim().split(/\s+/); + for (const [index, field] of fields.entries()) { + if (field.split(",").some((part) => /^\d+\//.test(part))) { + return `Invalid cron ${CRON_FIELD_BOUNDS[index].name} step: use * or a range before /`; + } + } return null; } catch (error) { return error instanceof Error ? error.message : "Invalid cron expression"; diff --git a/packages/server/src/server/schedule/cron.ts b/packages/server/src/server/schedule/cron.ts index 2ac2e78972..21491ff065 100644 --- a/packages/server/src/server/schedule/cron.ts +++ b/packages/server/src/server/schedule/cron.ts @@ -1,4 +1,7 @@ -import { parseCronExpression } from "@getpaseo/protocol/schedule/cron-expression"; +import { + parseCronExpression, + validateCronExpression, +} from "@getpaseo/protocol/schedule/cron-expression"; import type { ScheduleCadence } from "@getpaseo/protocol/schedule/types"; interface CronDateParts { @@ -78,7 +81,10 @@ function createCronDatePartsReader(timeZone: string | undefined): (date: Date) = export function validateScheduleCadence(cadence: ScheduleCadence): void { if (cadence.type === "cron") { - parseCronExpression(cadence.expression); + const error = validateCronExpression(cadence.expression); + if (error) { + throw new Error(error); + } if (cadence.timezone !== undefined) { assertValidTimeZone(cadence.timezone); } diff --git a/packages/server/src/server/schedule/service.test.ts b/packages/server/src/server/schedule/service.test.ts index 1a9f0f5fd4..53ce94cbee 100644 --- a/packages/server/src/server/schedule/service.test.ts +++ b/packages/server/src/server/schedule/service.test.ts @@ -2114,6 +2114,81 @@ describe("ScheduleService", () => { }); }); + test("rejects numeric cron steps when creating or updating a schedule", async () => { + const service = createScheduleService({ + paseoHome: tempDir, + logger: createTestLogger(), + agentManager: new AgentManager({ logger: createTestLogger() }), + agentStorage, + providerSnapshotManager: NO_UNATTENDED_SCHEDULE_POLICY, + now: () => now, + }); + const input = { + prompt: "Periodic check", + cadence: { type: "cron" as const, expression: "*/15 * * * *" }, + target: { + type: "new-agent" as const, + config: { provider: "claude" as const, cwd: tempDir }, + }, + runOnCreate: false, + }; + + await expect( + service.create({ ...input, cadence: { type: "cron", expression: "0/15 * * * *" } }), + ).rejects.toThrow("Invalid cron minute step: use * or a range before /"); + expect(await service.list()).toEqual([]); + + const created = await service.create(input); + await expect( + service.update({ id: created.id, cadence: { type: "cron", expression: "0/15 * * * *" } }), + ).rejects.toThrow("Invalid cron minute step: use * or a range before /"); + expect((await service.inspect(created.id)).cadence).toEqual(input.cadence); + }); + + test("recovers and runs numeric cron steps persisted by an older daemon", async () => { + const service = createScheduleService({ + paseoHome: tempDir, + logger: createTestLogger(), + agentManager: new AgentManager({ logger: createTestLogger() }), + agentStorage, + providerSnapshotManager: NO_UNATTENDED_SCHEDULE_POLICY, + now: () => now, + runner: async () => ({ agentId: null, output: "ok" }), + }); + const created = await service.create({ + prompt: "Periodic check", + cadence: { type: "cron", expression: "0 * * * *" }, + target: { + type: "new-agent", + config: { provider: "claude", cwd: tempDir }, + }, + runOnCreate: false, + }); + const store = new ScheduleStore(join(tempDir, "schedules"), createTestLogger()); + await store.update(created.id, (schedule) => ({ + ...schedule, + cadence: { type: "cron", expression: "0/15 * * * *" }, + })); + + now = new Date("2026-01-01T03:10:00.000Z"); + try { + await service.start(); + await service.stop(); + expect((await service.inspect(created.id)).nextRunAt).toBe("2026-01-01T04:00:00.000Z"); + + now = new Date("2026-01-01T04:00:00.000Z"); + await service.tick(); + const inspected = await service.inspect(created.id); + expect(inspected.cadence).toEqual({ type: "cron", expression: "0/15 * * * *" }); + expect(inspected.status).toBe("active"); + expect(inspected.nextRunAt).toBe("2026-01-01T05:00:00.000Z"); + expect(inspected.runs).toHaveLength(1); + expect(inspected.runs[0]).toMatchObject({ status: "succeeded", output: "ok" }); + } finally { + await service.stop(); + } + }); + test("advances stale nextRunAt on daemon restart", async () => { const service1 = createScheduleService({ paseoHome: tempDir,