From 0768a2efa68b72107f2f2e7fa0f0dc87fd12d3c4 Mon Sep 17 00:00:00 2001
From: salmonik <raisersoft@proton.me>
Date: Thu, 9 Jul 2026 16:20:43 +0200
Subject: fix cron double fire

Signed-off-by: salmonik <raisersoft@proton.me>
---
 src/tasks/mod.rs | 41 ++++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 13 deletions(-)

(limited to 'src/tasks')

diff --git a/src/tasks/mod.rs b/src/tasks/mod.rs
index f3f8811..1519c12 100644
--- a/src/tasks/mod.rs
+++ b/src/tasks/mod.rs
@@ -1,6 +1,6 @@
 use std::sync::Arc;
 
-use chrono::{Duration, Timelike, Utc};
+use chrono::{DateTime, Duration, Timelike, Utc};
 use cron::Schedule;
 use poise::serenity_prelude as serenity;
 use std::str::FromStr;
@@ -33,9 +33,6 @@ fn parse_cron(raw: &str) -> Result<Schedule, cron::error::Error> {
     Schedule::from_str(&normalized)
 }
 
-/// Check window: look for cron fires in the last WINDOW_SECONDS.
-/// 61s gives 1s overlap with 60s tick to avoid missing exact-boundary fires.
-const WINDOW_SECONDS: i64 = 61;
 /// Tick interval for the scheduler loop.
 const TICK_SECONDS: u64 = 60;
 
@@ -49,7 +46,6 @@ pub async fn check_and_execute_jobs(ctx: &CronContext) {
     };
 
     let now = Utc::now();
-    let window_start = now - Duration::seconds(WINDOW_SECONDS);
 
     for job in jobs {
         let schedule = match parse_cron(&job.cron_expression) {
@@ -60,26 +56,45 @@ pub async fn check_and_execute_jobs(ctx: &CronContext) {
             }
         };
 
-        // Check if any fire time fell within the last WINDOW_SECONDS.
-        let mut has_fired = false;
-        for next_fire in schedule.after(&window_start) {
+        // Find the next scheduled fire strictly after last_run.
+        // - If last_run is None (freshly created or never run), start from the
+        //   last fire at/after the previous tick boundary so we still catch the
+        //   fire scheduled for "now" on the first scan without doubling up.
+        // - If last_fire <= now, the job is due; we record last_run = last_fire
+        //   (not `now`) so a long execution delay can't skip a legitimately
+        //   pending later fire.
+        let reference: DateTime<Utc> = job.last_run.unwrap_or_else(|| {
+            // Step back a tick so the first scan after startup catches the fire
+            // scheduled for the minute we wake up on.
+            now - chrono::Duration::seconds(TICK_SECONDS as i64)
+        });
+
+        let mut due_fire: Option<DateTime<Utc>> = None;
+        for next_fire in schedule.after(&reference) {
             if next_fire > now {
                 break;
             }
-            has_fired = true;
-            break; // one fire per window is enough
+            due_fire = Some(next_fire);
+            break;
         }
 
-        if !has_fired {
+        let Some(fire_time) = due_fire else {
             continue;
-        }
+        };
 
-        info!("Executing cron job {}", job.id);
+        info!("Executing cron job {} (fire {})", job.id, fire_time);
         match job.r#type.as_str() {
             "audio" => execute_audio_job(ctx, &job).await,
             "wiadomość" => execute_message_job(ctx, &job).await,
             _ => {}
         }
+
+        // Record last_run so we never re-fire the same schedule instant on the
+        // next tick. Kept outside the match so a failed/empty send still marks
+        // the fire as consumed and doesn't retry on every tick.
+        if let Err(e) = database::update_cron_last_run(&ctx.pool, job.id, fire_time).await {
+            error!("Failed to update last_run for cron job {}: {e}", job.id);
+        }
     }
 }
 
-- 
cgit v1.2.3