Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/goose/src/agents/platform_tools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Expand Down
60 changes: 60 additions & 0 deletions crates/goose/src/cron_test.rs
Original file line number Diff line number Diff line change
@@ -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!();
}
}
}
3 changes: 3 additions & 0 deletions crates/goose/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,6 @@ pub mod temporal_scheduler;
pub mod token_counter;
pub mod tool_monitor;
pub mod tracing;

#[cfg(test)]
mod cron_test;
69 changes: 66 additions & 3 deletions crates/goose/src/scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()))?;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
47 changes: 44 additions & 3 deletions crates/goose/src/temporal_scheduler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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(),
};
Expand Down Expand Up @@ -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,
};
Expand Down Expand Up @@ -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");
}
}
Loading