From 0533bfa59d366d089f261878ccdd305a8a73d56f Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 11 Aug 2026 11:16:28 +0000 Subject: [PATCH] fix(cron): advance recurring jobs past the firing minute MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cron_next_from_expr returned the current matching minute, so after cron_poll updated next_run the job stayed due and re-fired every poll until the minute rolled — or forever when the next match was outside the 8-day search window. Start from the next minute and search up to a year; fail closed if next_run still cannot be computed. Co-authored-by: esadrianno --- src/tools/cron.c | 16 +++++++- src/tools/cron.h | 6 ++- tests/test_cron.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 113 insertions(+), 6 deletions(-) diff --git a/src/tools/cron.c b/src/tools/cron.c index eb46e18..6df6683 100644 --- a/src/tools/cron.c +++ b/src/tools/cron.c @@ -21,7 +21,8 @@ #define CRON_PREFIX_INTERVAL "interval:" #define CRON_PREFIX_AT "at:" #define CRON_PREFIX_CRON "cron:" -#define CRON_MAX_ITER_MINUTES (8 * 24 * 60) +/* Cover at least one leap year so monthly/yearly exprs can advance after firing. */ +#define CRON_MAX_ITER_MINUTES (366 * 24 * 60) static int parse_field(const char *s, int *out, int min_val, int max_val) { @@ -104,7 +105,14 @@ static long long cron_next_from_expr(const char *cron_part, long long now) { int fields[10]; if (parse_cron_expr(cron_part, fields) != 0) return -1; + /* + * Start at the beginning of the *next* minute. Returning the current + * minute would leave next_run <= now after cron_poll fires, so the job + * would re-deliver every poll (~1s) until the minute rolled over — and + * forever if the following match was outside the search window. + */ time_t t = (time_t)now; + t = t - (t % 60) + 60; struct tm tm; if (!localtime_r(&t, &tm)) return -1; int min = tm.tm_min, hour = tm.tm_hour, mday = tm.tm_mday, mon = tm.tm_mon + 1, wday = tm.tm_wday; @@ -184,8 +192,12 @@ static int cron_poll(channel_incoming_msg_t *out, int timeout_ms) cron_job_delete(row.id); } else { long long next = 0; - if (cron_parse_next_run(row.schedule, now, &next) == 0) + if (cron_parse_next_run(row.schedule, now, &next) == 0) { cron_job_update_next_run(row.id, next); + } else { + /* Fail closed: never leave a due next_run that re-fires every poll. */ + cron_job_update_next_run(row.id, now + 365LL * 24 * 3600); + } } memset(out, 0, sizeof(*out)); char session_id[256]; diff --git a/src/tools/cron.h b/src/tools/cron.h index 1635d89..1466be5 100644 --- a/src/tools/cron.h +++ b/src/tools/cron.h @@ -14,13 +14,15 @@ extern "C" { #endif /** - * Parse schedule and compute next_run from current time. + * Parse schedule and compute next_run strictly after `now`. * Formats: "cron:min hour dom month dow", "interval:N", "at:unix_ts". * Cron: 5 fields, * or N or N-M. dow 0-6 (Sun-Sat). + * Cron expressions start searching at the next minute boundary so a just-fired + * job cannot remain due in the same minute. * * @param schedule Schedule string. * @param now Current Unix timestamp. - * @param next_out Output: next run time. + * @param next_out Output: next run time (always > now on success for cron:/interval:). * @return 0 on success, -1 on parse error. */ int cron_parse_next_run(const char *schedule, long long now, long long *next_out); diff --git a/tests/test_cron.c b/tests/test_cron.c index ea21c0c..515864a 100644 --- a/tests/test_cron.c +++ b/tests/test_cron.c @@ -3,6 +3,8 @@ * @brief Unit tests for cron: schedule parsing, next_run, one-shot. */ +#define _POSIX_C_SOURCE 200809L + #include "tools/cron.h" #include "core/memory.h" #include @@ -41,7 +43,7 @@ static int test_cron_expr_next_run(void) long long now = 1700000000; long long next = 0; ASSERT(cron_parse_next_run("0 0 * * *", now, &next) == 0); - ASSERT(next >= now); + ASSERT(next > now); return 0; } @@ -50,7 +52,95 @@ static int test_cron_expr_with_prefix(void) long long now = 1700000000; long long next = 0; ASSERT(cron_parse_next_run("cron:0 0 * * *", now, &next) == 0); - ASSERT(next >= now); + ASSERT(next > now); + return 0; +} + +static int test_cron_expr_skips_current_minute(void) +{ + long long now = 1700000017; /* mid-minute so a same-minute match would be <= now */ + time_t t = (time_t)now; + struct tm tm; + ASSERT(localtime_r(&t, &tm) != NULL); + char schedule[64]; + snprintf(schedule, sizeof(schedule), "cron:%d %d * * *", tm.tm_min, tm.tm_hour); + long long next = 0; + ASSERT(cron_parse_next_run(schedule, now, &next) == 0); + ASSERT(next > now); + /* Must advance at least to the next minute boundary (not remain due this minute). */ + ASSERT(next >= (now - (now % 60) + 60)); + time_t next_t = (time_t)next; + struct tm next_tm; + ASSERT(localtime_r(&next_t, &next_tm) != NULL); + ASSERT(next_tm.tm_min == tm.tm_min); + ASSERT(next_tm.tm_hour == tm.tm_hour); + return 0; +} + +static int test_cron_expr_monthly_beyond_eight_days(void) +{ + /* Anchor mid-month so the 1st of next month is > 8 days away. */ + time_t t = 1700000000; /* 2023-11-14 local-ish depending on TZ */ + struct tm tm; + ASSERT(localtime_r(&t, &tm) != NULL); + tm.tm_mday = 15; + tm.tm_hour = 12; + tm.tm_min = 0; + tm.tm_sec = 0; + tm.tm_isdst = -1; + t = mktime(&tm); + ASSERT(t != (time_t)-1); + long long now = (long long)t; + long long next = 0; + ASSERT(cron_parse_next_run("cron:0 0 1 * *", now, &next) == 0); + ASSERT(next > now); + ASSERT(next - now > 8LL * 24 * 3600); + time_t next_t = (time_t)next; + struct tm next_tm; + ASSERT(localtime_r(&next_t, &next_tm) != NULL); + ASSERT(next_tm.tm_mday == 1); + ASSERT(next_tm.tm_hour == 0); + ASSERT(next_tm.tm_min == 0); + return 0; +} + +static int test_cron_poll_advances_recurring_past_due_minute(void) +{ + const char *path = "/tmp/shellclaw_test_cron_poll_advance.db"; + remove(path); + ASSERT(memory_init(path) == 0); + + long long now = (long long)time(NULL); + time_t t = (time_t)now; + struct tm tm; + ASSERT(localtime_r(&t, &tm) != NULL); + char schedule[64]; + snprintf(schedule, sizeof(schedule), "cron:%d %d * * *", tm.tm_min, tm.tm_hour); + + ASSERT(cron_job_create("poll_adv1", schedule, "due now", "cli", "default", now - 1, 1) == 0); + + const channel_t *cron_ch = channel_cron_get(); + ASSERT(cron_ch != NULL && cron_ch->poll != NULL); + channel_incoming_msg_t msg; + memset(&msg, 0, sizeof(msg)); + ASSERT(cron_ch->poll(&msg, 0) == 1); + ASSERT(msg.text != NULL); + free(msg.session_id); + free(msg.user_id); + free(msg.text); + + cron_job_row_t rows[4]; + ASSERT(cron_job_list(rows, 4) == 1); + ASSERT(strcmp(rows[0].id, "poll_adv1") == 0); + ASSERT(rows[0].next_run > now); + ASSERT(rows[0].next_run >= (now - (now % 60) + 60)); + + memset(&msg, 0, sizeof(msg)); + ASSERT(cron_ch->poll(&msg, 0) == 0); + + cron_job_delete("poll_adv1"); + memory_cleanup(); + remove(path); return 0; } @@ -145,6 +235,9 @@ int main(void) RUN(test_at_one_shot()); RUN(test_cron_expr_next_run()); RUN(test_cron_expr_with_prefix()); + RUN(test_cron_expr_skips_current_minute()); + RUN(test_cron_expr_monthly_beyond_eight_days()); + RUN(test_cron_poll_advances_recurring_past_due_minute()); RUN(test_invalid_schedule()); RUN(test_cron_job_crud_and_due()); RUN(test_cron_tool_execute());