fix cron parsing for windows (#3044)

crates/goose/src/agents/platform_tools.rs

@@ -143,7 +143,7 @@ pub fn manage_schedule_tool() -> Tool {
                 },
                 "job_id": {"type": "string", "description": "Job identifier for operations on existing jobs"},
                 "recipe_path": {"type": "string", "description": "Path to recipe file for create action"},
-                "cron_expression": {"type": "string", "description": "A six field cron expression for create action"},
+                "cron_expression": {"type": "string", "description": "A cron expression for create action. Supports both 5-field (minute hour day month weekday) and 6-field (second minute hour day month weekday) formats. 5-field expressions are automatically converted to 6-field by prepending '0' for seconds."},
                 "execution_mode": {"type": "string", "description": "Execution mode for create action: 'foreground' or 'background'", "enum": ["foreground", "background"], "default": "background"},
                 "limit": {"type": "integer", "description": "Limit for sessions list", "default": 50},
                 "session_id": {"type": "string", "description": "Session identifier for session_content action"}

crates/goose/src/cron_test.rs

@@ -0,0 +1,60 @@
+#[cfg(test)]
+mod cron_parsing_tests {
+    use crate::scheduler::normalize_cron_expression;
+    use tokio_cron_scheduler::Job;
+
+    #[test]
+    fn test_normalize_cron_expression() {
+        // Test 5-field to 6-field conversion
+        assert_eq!(normalize_cron_expression("0 12 * * *"), "0 0 12 * * *");
+        assert_eq!(normalize_cron_expression("*/5 * * * *"), "0 */5 * * * *");
+        assert_eq!(normalize_cron_expression("0 0 * * 1"), "0 0 0 * * 1");
+
+        // Test 6-field expressions (should remain unchanged)
+        assert_eq!(normalize_cron_expression("0 0 12 * * *"), "0 0 12 * * *");
+        assert_eq!(
+            normalize_cron_expression("*/30 */5 * * * *"),
+            "*/30 */5 * * * *"
+        );
+
+        // Test invalid expressions (should remain unchanged but warn)
+        assert_eq!(normalize_cron_expression("* * *"), "* * *");
+        assert_eq!(normalize_cron_expression("* * * * * * *"), "* * * * * * *");
+        assert_eq!(normalize_cron_expression(""), "");
+    }
+
+    #[tokio::test]
+    async fn test_cron_expression_formats() {
+        // Test different cron formats to see which ones work
+        let test_expressions = vec![
+            ("0 0 * * *", "5-field: every day at midnight"),
+            ("0 0 0 * * *", "6-field: every day at midnight"),
+            ("* * * * *", "5-field: every minute"),
+            ("* * * * * *", "6-field: every second"),
+            ("0 */5 * * *", "5-field: every 5 minutes"),
+            ("0 0 */5 * * *", "6-field: every 5 minutes"),
+            ("0 0 12 * * *", "6-field: every day at noon"),
+            ("0 12 * * *", "5-field: every day at noon"),
+        ];
+
+        for (expr, desc) in test_expressions {
+            println!("Testing cron expression: '{}' ({})", expr, desc);
+            let expr_owned = expr.to_string();
+
+            // Test with normalization
+            let normalized = normalize_cron_expression(expr);
+            println!("  Normalized to: '{}'", normalized);
+
+            match Job::new_async(&normalized, move |_uuid, _l| {
+                let expr_clone = expr_owned.clone();
+                Box::pin(async move {
+                    println!("Job executed for: {}", expr_clone);
+                })
+            }) {
+                Ok(_) => println!("  ✅ Successfully parsed normalized: '{}'", normalized),
+                Err(e) => println!("  ❌ Failed to parse normalized '{}': {}", normalized, e),
+            }
+            println!();
+        }
+    }
+}

crates/goose/src/lib.rs

@@ -15,3 +15,6 @@ pub mod temporal_scheduler;
 pub mod token_counter;
 pub mod tool_monitor;
 pub mod tracing;
+
+#[cfg(test)]
+mod cron_test;

crates/goose/src/scheduler.rs

@@ -27,6 +27,34 @@ use crate::session::storage::SessionMetadata;
 type RunningTasksMap = HashMap<String, tokio::task::AbortHandle>;
 type JobsMap = HashMap<String, (JobId, ScheduledJob)>;
 
+/// Converts a 5-field cron expression to a 6-field expression by prepending "0" for seconds
+/// If the expression already has 6 fields, returns it unchanged
+/// If the expression is invalid, returns the original expression
+pub fn normalize_cron_expression(cron_expr: &str) -> String {
+    let fields: Vec<&str> = cron_expr.split_whitespace().collect();
+
+    match fields.len() {
+        5 => {
+            // 5-field cron: minute hour day month weekday
+            // Convert to 6-field: second minute hour day month weekday
+            format!("0 {}", cron_expr)
+        }
+        6 => {
+            // Already 6-field, return as-is
+            cron_expr.to_string()
+        }
+        _ => {
+            // Invalid number of fields, return original (will likely fail parsing later)
+            tracing::warn!(
+                "Invalid cron expression '{}': expected 5 or 6 fields, got {}",
+                cron_expr,
+                fields.len()
+            );
+            cron_expr.to_string()
+        }
+    }
+}
+
 pub fn get_default_scheduler_storage_path() -> Result<PathBuf, io::Error> {
     let strategy = choose_app_strategy(config::APP_STRATEGY.clone())
         .map_err(|e| io::Error::new(io::ErrorKind::NotFound, e.to_string()))?;
@@ -233,7 +261,16 @@ impl Scheduler {
         let storage_path_for_task = self.storage_path.clone();
         let running_tasks_for_task = self.running_tasks.clone();
 
-        let cron_task = Job::new_async(&stored_job.cron, move |_uuid, _l| {
+        tracing::info!("Attempting to parse cron expression: '{}'", stored_job.cron);
+        let normalized_cron = normalize_cron_expression(&stored_job.cron);
+        if normalized_cron != stored_job.cron {
+            tracing::info!(
+                "Normalized cron expression from '{}' to '{}'",
+                stored_job.cron,
+                normalized_cron
+            );
+        }
+        let cron_task = Job::new_async(&normalized_cron, move |_uuid, _l| {
             let task_job_id = job_for_task.id.clone();
             let current_jobs_arc = jobs_arc_for_task.clone();
             let local_storage_path = storage_path_for_task.clone();
@@ -389,7 +426,20 @@ impl Scheduler {
             let storage_path_for_task = self.storage_path.clone();
             let running_tasks_for_task = self.running_tasks.clone();
 
-            let cron_task = Job::new_async(&job_to_load.cron, move |_uuid, _l| {
+            tracing::info!(
+                "Loading job '{}' with cron expression: '{}'",
+                job_to_load.id,
+                job_to_load.cron
+            );
+            let normalized_cron = normalize_cron_expression(&job_to_load.cron);
+            if normalized_cron != job_to_load.cron {
+                tracing::info!(
+                    "Normalized cron expression from '{}' to '{}'",
+                    job_to_load.cron,
+                    normalized_cron
+                );
+            }
+            let cron_task = Job::new_async(&normalized_cron, move |_uuid, _l| {
                 let task_job_id = job_for_task.id.clone();
                 let current_jobs_arc = jobs_arc_for_task.clone();
                 let local_storage_path = storage_path_for_task.clone();
@@ -747,7 +797,20 @@ impl Scheduler {
                 let storage_path_for_task = self.storage_path.clone();
                 let running_tasks_for_task = self.running_tasks.clone();
 
-                let cron_task = Job::new_async(&new_cron, move |_uuid, _l| {
+                tracing::info!(
+                    "Updating job '{}' with new cron expression: '{}'",
+                    sched_id,
+                    new_cron
+                );
+                let normalized_cron = normalize_cron_expression(&new_cron);
+                if normalized_cron != new_cron {
+                    tracing::info!(
+                        "Normalized cron expression from '{}' to '{}'",
+                        new_cron,
+                        normalized_cron
+                    );
+                }
+                let cron_task = Job::new_async(&normalized_cron, move |_uuid, _l| {
                     let task_job_id = job_for_task.id.clone();
                     let current_jobs_arc = jobs_arc_for_task.clone();
                     let local_storage_path = storage_path_for_task.clone();

crates/goose/src/temporal_scheduler.rs

@@ -9,7 +9,7 @@ use serde::{Deserialize, Serialize};
 use tokio::time::sleep;
 use tracing::{info, warn};
 
-use crate::scheduler::{ScheduledJob, SchedulerError};
+use crate::scheduler::{normalize_cron_expression, ScheduledJob, SchedulerError};
 use crate::scheduler_trait::SchedulerTrait;
 use crate::session::storage::SessionMetadata;
 
@@ -487,10 +487,21 @@ impl TemporalScheduler {
             "TemporalScheduler: add_scheduled_job() called for job '{}'",
             job.id
         );
+
+        // Normalize the cron expression to ensure it's 6-field format
+        let normalized_cron = normalize_cron_expression(&job.cron);
+        if normalized_cron != job.cron {
+            tracing::info!(
+                "TemporalScheduler: Normalized cron expression from '{}' to '{}'",
+                job.cron,
+                normalized_cron
+            );
+        }
+
         let request = JobRequest {
             action: "create".to_string(),
             job_id: Some(job.id.clone()),
-            cron: Some(job.cron.clone()),
+            cron: Some(normalized_cron),
             recipe_path: Some(job.source.clone()),
             execution_mode: job.execution_mode.clone(),
         };
@@ -690,10 +701,20 @@ impl TemporalScheduler {
             new_cron
         );
 
+        // Normalize the cron expression to ensure it's 6-field format
+        let normalized_cron = normalize_cron_expression(&new_cron);
+        if normalized_cron != new_cron {
+            tracing::info!(
+                "TemporalScheduler: Normalized cron expression from '{}' to '{}'",
+                new_cron,
+                normalized_cron
+            );
+        }
+
         let request = JobRequest {
             action: "update".to_string(),
             job_id: Some(sched_id.to_string()),
-            cron: Some(new_cron),
+            cron: Some(normalized_cron),
             recipe_path: None,
             execution_mode: None,
         };
@@ -1284,4 +1305,24 @@ mod tests {
             }
         }
     }
+
+    #[test]
+    fn test_cron_normalization_in_temporal_scheduler() {
+        // Test that the temporal scheduler uses cron normalization correctly
+        use crate::scheduler::normalize_cron_expression;
+
+        // Test cases that should be normalized
+        assert_eq!(normalize_cron_expression("0 12 * * *"), "0 0 12 * * *");
+        assert_eq!(normalize_cron_expression("*/5 * * * *"), "0 */5 * * * *");
+        assert_eq!(normalize_cron_expression("0 0 * * 1"), "0 0 0 * * 1");
+
+        // Test cases that should remain unchanged
+        assert_eq!(normalize_cron_expression("0 0 12 * * *"), "0 0 12 * * *");
+        assert_eq!(
+            normalize_cron_expression("*/30 */5 * * * *"),
+            "*/30 */5 * * * *"
+        );
+
+        println!("✅ Cron normalization works correctly in TemporalScheduler");
+    }
 }