fix cron double fire

Signed-off-by: salmonik <raisersoft@proton.me>

2 files changed, 46 insertions, 14 deletions

diff --git a/src/database/mod.rs b/src/database/mod.rs
index 53f4bf4..e803da9 100644
--- a/src/database/mod.rs
+++ b/src/database/mod.rs
@@ -50,12 +50,19 @@ async fn run_migrations(pool: &DbPool) -> Result<()> {
             active INTEGER NOT NULL DEFAULT 1,
             audio_filename TEXT,
             message TEXT,
-            created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP
+            created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
+            last_run TIMESTAMP
         )",
     )
     .execute(pool)
     .await?;
 
+    // Add last_run column for pre-existing databases that lack it.
+    sqlx::query("ALTER TABLE cron_jobs ADD COLUMN last_run TIMESTAMP")
+        .execute(pool)
+        .await
+        .ok();
+
     sqlx::query(
         "CREATE TABLE IF NOT EXISTS triggers (
             id INTEGER PRIMARY KEY AUTOINCREMENT,
@@ -157,6 +164,7 @@ pub struct CronJob {
     pub message: Option<String>,
     #[allow(dead_code)]
     pub created_at: Option<DateTime<Utc>>,
+    pub last_run: Option<DateTime<Utc>>,
 }
 
 #[allow(dead_code)]
@@ -202,6 +210,15 @@ pub async fn delete_cron_job(pool: &DbPool, id: i64) -> Result<bool> {
     Ok(result.rows_affected() > 0)
 }
 
+pub async fn update_cron_last_run(pool: &DbPool, id: i64, last_run: DateTime<Utc>) -> Result<()> {
+    sqlx::query("UPDATE cron_jobs SET last_run = ? WHERE id = ?")
+        .bind(last_run)
+        .bind(id)
+        .execute(pool)
+        .await?;
+    Ok(())
+}
+
 pub async fn set_cron_job_active(pool: &DbPool, id: i64, active: bool) -> Result<bool> {
     let result = sqlx::query("UPDATE cron_jobs SET active = ? WHERE id = ?")
         .bind(if active { 1 } else { 0 })
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);
+        }
     }
 }