From eff8fe00c6ef261fa9d8645c0941dec090027495 Mon Sep 17 00:00:00 2001 From: YellowSnnowmann <167776381+YellowSnnowmann@users.noreply.github.com> Date: Wed, 29 Apr 2026 07:28:16 +0530 Subject: [PATCH] Fix:Issue 921 cron improvements (#975) Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: Steven Enamakel <31011319+senamakel@users.noreply.github.com> --- src/openhuman/cron/mod.rs | 4 +- src/openhuman/cron/ops.rs | 10 +- src/openhuman/cron/ops_tests.rs | 45 ++++ src/openhuman/cron/schedule.rs | 273 ++++++++++++++++++-- src/openhuman/cron/scheduler_tests.rs | 63 ++++- src/openhuman/cron/seed.rs | 5 +- src/openhuman/cron/store.rs | 2 + src/openhuman/cron/store_tests.rs | 34 +++ src/openhuman/cron/types.rs | 27 ++ src/openhuman/tools/impl/cron/add.rs | 89 ++++++- src/openhuman/tools/impl/system/schedule.rs | 1 + 11 files changed, 523 insertions(+), 30 deletions(-) diff --git a/src/openhuman/cron/mod.rs b/src/openhuman/cron/mod.rs index 3145cc398..c5bb91c46 100644 --- a/src/openhuman/cron/mod.rs +++ b/src/openhuman/cron/mod.rs @@ -24,4 +24,6 @@ pub use store::{ list_jobs, list_runs, record_last_run, record_run, remove_job, reschedule_after_run, update_job, }; -pub use types::{CronJob, CronJobPatch, CronRun, DeliveryConfig, JobType, Schedule, SessionTarget}; +pub use types::{ + ActiveHours, CronJob, CronJobPatch, CronRun, DeliveryConfig, JobType, Schedule, SessionTarget, +}; diff --git a/src/openhuman/cron/ops.rs b/src/openhuman/cron/ops.rs index d5dcca648..571a077ea 100644 --- a/src/openhuman/cron/ops.rs +++ b/src/openhuman/cron/ops.rs @@ -45,6 +45,10 @@ pub fn resume_job(config: &Config, id: &str) -> Result { } /// Update an existing cron job using the same rules as the legacy CLI, but without CLI wiring. +/// +/// `expression` and `tz` are merged with the existing [`Schedule::Cron`] fields; the +/// existing `active_hours` is always preserved as-is. To set or clear `active_hours` +/// directly, use the RPC path (`cron.update` with a full [`CronJobPatch`]). pub fn update_cron_job( config: &Config, id: &str, @@ -61,16 +65,18 @@ pub fn update_cron_job( // tz alone updates the timezone and expression alone preserves the timezone. let schedule = if expression.is_some() || tz.is_some() { let existing = get_job(config, id)?; - let (existing_expr, existing_tz) = match existing.schedule { + let (existing_expr, existing_tz, existing_active) = match existing.schedule { Schedule::Cron { expr, tz: existing_tz, - } => (expr, existing_tz), + active_hours: existing_active, + } => (expr, existing_tz, existing_active), _ => anyhow::bail!("Cannot update expression/tz on a non-cron schedule"), }; Some(Schedule::Cron { expr: expression.unwrap_or(existing_expr), tz: tz.or(existing_tz), + active_hours: existing_active, }) } else { None diff --git a/src/openhuman/cron/ops_tests.rs b/src/openhuman/cron/ops_tests.rs index 16a1d80db..d07b95c89 100644 --- a/src/openhuman/cron/ops_tests.rs +++ b/src/openhuman/cron/ops_tests.rs @@ -1,4 +1,5 @@ use super::*; +use crate::openhuman::cron::ActiveHours; use tempfile::TempDir; fn test_config(tmp: &TempDir) -> Config { @@ -18,6 +19,7 @@ fn make_job(config: &Config, expr: &str, tz: Option<&str>, cmd: &str) -> CronJob Schedule::Cron { expr: expr.into(), tz: tz.map(Into::into), + active_hours: None, }, cmd, ) @@ -102,6 +104,7 @@ fn update_tz_alone_sets_timezone() { Schedule::Cron { expr: "*/5 * * * *".into(), tz: Some("America/Los_Angeles".into()), + active_hours: None, } ); } @@ -120,6 +123,48 @@ fn update_expr_alone_preserves_timezone() { Schedule::Cron { expr: "0 10 * * *".into(), tz: Some("UTC".into()), + active_hours: None, + } + ); +} + +#[test] +fn update_expr_and_tz_preserve_active_hours() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + let active_hours = ActiveHours { + start: "09:00".into(), + end: "17:00".into(), + }; + let job = add_shell_job( + &config, + None, + Schedule::Cron { + expr: "*/5 * * * *".into(), + tz: Some("UTC".into()), + active_hours: Some(active_hours.clone()), + }, + "echo test", + ) + .unwrap(); + + run_update( + &config, + &job.id, + Some("0 10 * * *"), + Some("America/Los_Angeles"), + None, + None, + ) + .unwrap(); + + let updated = get_job(&config, &job.id).unwrap(); + assert_eq!( + updated.schedule, + Schedule::Cron { + expr: "0 10 * * *".into(), + tz: Some("America/Los_Angeles".into()), + active_hours: Some(active_hours), } ); } diff --git a/src/openhuman/cron/schedule.rs b/src/openhuman/cron/schedule.rs index f1519b558..d415096c1 100644 --- a/src/openhuman/cron/schedule.rs +++ b/src/openhuman/cron/schedule.rs @@ -1,29 +1,48 @@ -use crate::openhuman::cron::Schedule; +use crate::openhuman::cron::{ActiveHours, Schedule}; use anyhow::{Context, Result}; -use chrono::{DateTime, Duration as ChronoDuration, Utc}; +use chrono::{DateTime, Duration as ChronoDuration, NaiveTime, Timelike, Utc}; use cron::Schedule as CronExprSchedule; use std::str::FromStr; pub fn next_run_for_schedule(schedule: &Schedule, from: DateTime) -> Result> { match schedule { - Schedule::Cron { expr, tz } => { + Schedule::Cron { + expr, + tz, + active_hours, + } => { let normalized = normalize_expression(expr)?; let cron = CronExprSchedule::from_str(&normalized) .with_context(|| format!("Invalid cron expression: {expr}"))?; + let timezone = ScheduleTimeZone::parse(tz.as_deref())?; + // Parsing is cheap; validated at job-creation time via validate_schedule. + let active_window = active_hours.as_ref().map(ActiveWindow::parse).transpose()?; - if let Some(tz_name) = tz { - let timezone = chrono_tz::Tz::from_str(tz_name) - .with_context(|| format!("Invalid IANA timezone: {tz_name}"))?; - let localized_from = from.with_timezone(&timezone); - let next_local = cron.after(&localized_from).next().ok_or_else(|| { - anyhow::anyhow!("No future occurrence for expression: {expr}") - })?; - Ok(next_local.with_timezone(&Utc)) - } else { - cron.after(&from) - .next() - .ok_or_else(|| anyhow::anyhow!("No future occurrence for expression: {expr}")) + let mut current_from = from; + for _ in 0..100_000 { + let next_utc = timezone.next_after(&cron, current_from, expr)?; + + if let Some(active) = &active_window { + let local_t = timezone.local_time_of_day(next_utc); + if active.contains(local_t) { + return Ok(next_utc); + } + tracing::debug!( + "[cron] next_run candidate {} outside active window {}–{}, advancing", + next_utc, + active.start, + active.end + ); + current_from = next_utc; + } else { + return Ok(next_utc); + } } + tracing::warn!( + "[cron] no occurrence found within active_hours for expr={} after 100,000 candidates", + expr + ); + anyhow::bail!("No future occurrence found within active hours after 100,000 attempts") } Schedule::At { at } => Ok(*at), Schedule::Every { every_ms } => { @@ -40,8 +59,16 @@ pub fn next_run_for_schedule(schedule: &Schedule, from: DateTime) -> Result pub fn validate_schedule(schedule: &Schedule, now: DateTime) -> Result<()> { match schedule { - Schedule::Cron { expr, .. } => { + Schedule::Cron { + expr, + tz, + active_hours, + } => { let _ = normalize_expression(expr)?; + if let Some(active) = active_hours { + let _ = ActiveWindow::parse(active)?; + } + let _ = ScheduleTimeZone::parse(tz.as_deref())?; let _ = next_run_for_schedule(schedule, now)?; Ok(()) } @@ -67,6 +94,87 @@ pub fn schedule_cron_expression(schedule: &Schedule) -> Option { } } +#[derive(Debug, Clone, Copy)] +enum ScheduleTimeZone { + Local, + Named(chrono_tz::Tz), +} + +impl ScheduleTimeZone { + fn parse(tz: Option<&str>) -> Result { + match tz { + Some(tz_name) => chrono_tz::Tz::from_str(tz_name) + .map(Self::Named) + .with_context(|| format!("Invalid IANA timezone: {tz_name}")), + None => Ok(Self::Local), + } + } + + fn next_after( + self, + cron: &CronExprSchedule, + from: DateTime, + expr: &str, + ) -> Result> { + match self { + Self::Named(timezone) => { + let localized_from = from.with_timezone(&timezone); + let next_local = cron.after(&localized_from).next().ok_or_else(|| { + anyhow::anyhow!("No future occurrence for expression: {expr}") + })?; + Ok(next_local.with_timezone(&Utc)) + } + Self::Local => { + let localized_from = from.with_timezone(&chrono::Local); + let next_local = cron.after(&localized_from).next().ok_or_else(|| { + anyhow::anyhow!("No future occurrence for expression: {expr}") + })?; + Ok(next_local.with_timezone(&Utc)) + } + } + } + + fn local_time_of_day(self, time: DateTime) -> NaiveTime { + match self { + Self::Named(timezone) => { + let localized = time.with_timezone(&timezone); + NaiveTime::from_hms_opt(localized.hour(), localized.minute(), 0) + .expect("hour() and minute() from a valid DateTime are always in-range") + } + Self::Local => { + let localized = time.with_timezone(&chrono::Local); + NaiveTime::from_hms_opt(localized.hour(), localized.minute(), 0) + .expect("hour() and minute() from a valid DateTime are always in-range") + } + } + } +} + +#[derive(Debug, Clone, Copy)] +struct ActiveWindow { + start: NaiveTime, + end: NaiveTime, +} + +impl ActiveWindow { + fn parse(active: &ActiveHours) -> Result { + let start = NaiveTime::parse_from_str(&active.start, "%H:%M") + .with_context(|| format!("Invalid active_hours.start: {}", active.start))?; + let end = NaiveTime::parse_from_str(&active.end, "%H:%M") + .with_context(|| format!("Invalid active_hours.end: {}", active.end))?; + Ok(Self { start, end }) + } + + fn contains(self, time: NaiveTime) -> bool { + if self.start <= self.end { + time >= self.start && time <= self.end + } else { + // Window spans midnight (e.g. 22:00 to 06:00). + time >= self.start || time <= self.end + } + } +} + pub fn normalize_expression(expression: &str) -> Result { let expression = expression.trim(); let field_count = expression.split_whitespace().count(); @@ -106,6 +214,7 @@ mod tests { let schedule = Schedule::Cron { expr: "0 9 * * *".into(), tz: Some("America/Los_Angeles".into()), + active_hours: None, }; let next = next_run_for_schedule(&schedule, from).unwrap(); @@ -155,15 +264,26 @@ mod tests { // ── next_run_for_schedule ─────────────────────────────────────── #[test] - fn next_run_cron_without_tz_uses_utc_by_default() { - // 0 9 * * * at 2026-02-16 00:00Z → next UTC 9am same day. - let from = Utc.with_ymd_and_hms(2026, 2, 16, 0, 0, 0).unwrap(); + fn next_run_cron_without_tz_uses_local_by_default() { + // Express `from` as local midnight so the expected next-09:00 is always on the + // same calendar day, regardless of the host timezone. A UTC-fixed `from` would + // land at different local times on different machines (e.g. already 10:00 local + // on a UTC+10 host), making the expected date machine-dependent. + let from_local = chrono::Local + .with_ymd_and_hms(2026, 2, 16, 0, 0, 0) + .unwrap(); + let from = from_local.with_timezone(&Utc); let schedule = Schedule::Cron { expr: "0 9 * * *".into(), tz: None, + active_hours: None, }; let next = next_run_for_schedule(&schedule, from).unwrap(); - assert_eq!(next, Utc.with_ymd_and_hms(2026, 2, 16, 9, 0, 0).unwrap()); + + let expected_local = chrono::Local + .with_ymd_and_hms(2026, 2, 16, 9, 0, 0) + .unwrap(); + assert_eq!(next, expected_local.with_timezone(&Utc)); } #[test] @@ -171,6 +291,7 @@ mod tests { let schedule = Schedule::Cron { expr: "not a cron".into(), tz: None, + active_hours: None, }; let err = next_run_for_schedule(&schedule, Utc::now()).unwrap_err(); assert!(err.to_string().to_lowercase().contains("invalid")); @@ -181,6 +302,7 @@ mod tests { let schedule = Schedule::Cron { expr: "0 9 * * *".into(), tz: Some("Not/A_Real_Tz".into()), + active_hours: None, }; let err = next_run_for_schedule(&schedule, Utc::now()).unwrap_err(); assert!(err @@ -235,6 +357,7 @@ mod tests { let schedule = Schedule::Cron { expr: "*/5 * * * *".into(), tz: None, + active_hours: None, }; assert!(validate_schedule(&schedule, now).is_ok()); } @@ -244,6 +367,7 @@ mod tests { let schedule = Schedule::Cron { expr: "not a cron".into(), tz: None, + active_hours: None, }; assert!(validate_schedule(&schedule, Utc::now()).is_err()); } @@ -255,6 +379,7 @@ mod tests { let s = Schedule::Cron { expr: "0 9 * * *".into(), tz: Some("UTC".into()), + active_hours: None, }; assert_eq!(schedule_cron_expression(&s).as_deref(), Some("0 9 * * *")); } @@ -264,4 +389,112 @@ mod tests { assert!(schedule_cron_expression(&Schedule::Every { every_ms: 1000 }).is_none()); assert!(schedule_cron_expression(&Schedule::At { at: Utc::now() }).is_none()); } + + #[test] + fn next_run_respects_active_hours() { + // Schedule: every minute + // Active hours: 09:00 - 09:05 + let schedule = Schedule::Cron { + expr: "* * * * *".into(), + tz: Some("UTC".into()), + active_hours: Some(ActiveHours { + start: "09:00".into(), + end: "09:05".into(), + }), + }; + + // If it's 08:00, next run should be 09:00 + let from = Utc.with_ymd_and_hms(2026, 2, 16, 8, 0, 0).unwrap(); + let next = next_run_for_schedule(&schedule, from).unwrap(); + assert_eq!(next, Utc.with_ymd_and_hms(2026, 2, 16, 9, 0, 0).unwrap()); + + // If it's 09:02, next run should be 09:03 + let from = Utc.with_ymd_and_hms(2026, 2, 16, 9, 2, 0).unwrap(); + let next = next_run_for_schedule(&schedule, from).unwrap(); + assert_eq!(next, Utc.with_ymd_and_hms(2026, 2, 16, 9, 3, 0).unwrap()); + + // If it's 09:05, next run should be 09:00 NEXT DAY + let from = Utc.with_ymd_and_hms(2026, 2, 16, 9, 5, 0).unwrap(); + let next = next_run_for_schedule(&schedule, from).unwrap(); + assert_eq!(next, Utc.with_ymd_and_hms(2026, 2, 17, 9, 0, 0).unwrap()); + } + + #[test] + fn next_run_respects_active_hours_spanning_midnight() { + // Active hours: 22:00 - 02:00 + let schedule = Schedule::Cron { + expr: "0 * * * *".into(), // every hour + tz: Some("UTC".into()), + active_hours: Some(ActiveHours { + start: "22:00".into(), + end: "02:00".into(), + }), + }; + + // 20:00 -> 22:00 + let from = Utc.with_ymd_and_hms(2026, 2, 16, 20, 0, 0).unwrap(); + let next = next_run_for_schedule(&schedule, from).unwrap(); + assert_eq!(next, Utc.with_ymd_and_hms(2026, 2, 16, 22, 0, 0).unwrap()); + + // 23:00 -> 00:00 + let from = Utc.with_ymd_and_hms(2026, 2, 16, 23, 0, 0).unwrap(); + let next = next_run_for_schedule(&schedule, from).unwrap(); + assert_eq!(next, Utc.with_ymd_and_hms(2026, 2, 17, 0, 0, 0).unwrap()); + + // 01:00 -> 02:00 + let from = Utc.with_ymd_and_hms(2026, 2, 17, 1, 0, 0).unwrap(); + let next = next_run_for_schedule(&schedule, from).unwrap(); + assert_eq!(next, Utc.with_ymd_and_hms(2026, 2, 17, 2, 0, 0).unwrap()); + + // 03:00 -> 22:00 SAME DAY (since it's early morning) + let from = Utc.with_ymd_and_hms(2026, 2, 17, 3, 0, 0).unwrap(); + let next = next_run_for_schedule(&schedule, from).unwrap(); + assert_eq!(next, Utc.with_ymd_and_hms(2026, 2, 17, 22, 0, 0).unwrap()); + } + + #[test] + fn next_run_respects_active_hours_in_schedule_timezone() { + let schedule = Schedule::Cron { + expr: "0 * * * *".into(), + tz: Some("America/Los_Angeles".into()), + active_hours: Some(ActiveHours { + start: "09:00".into(), + end: "10:00".into(), + }), + }; + + let from = Utc.with_ymd_and_hms(2026, 2, 16, 15, 30, 0).unwrap(); + let next = next_run_for_schedule(&schedule, from).unwrap(); + + assert_eq!(next, Utc.with_ymd_and_hms(2026, 2, 16, 17, 0, 0).unwrap()); + } + + #[test] + fn validate_schedule_rejects_invalid_active_hours() { + let now = Utc::now(); + let schedule = Schedule::Cron { + expr: "* * * * *".into(), + tz: None, + active_hours: Some(ActiveHours { + start: "invalid".into(), + end: "09:00".into(), + }), + }; + assert!(validate_schedule(&schedule, now).is_err()); + } + + #[test] + fn validate_schedule_rejects_invalid_active_hours_end() { + let now = Utc::now(); + let schedule = Schedule::Cron { + expr: "* * * * *".into(), + tz: Some("UTC".into()), + active_hours: Some(ActiveHours { + start: "09:00".into(), + end: "24:00".into(), + }), + }; + let err = validate_schedule(&schedule, now).unwrap_err(); + assert!(err.to_string().contains("active_hours.end")); + } } diff --git a/src/openhuman/cron/scheduler_tests.rs b/src/openhuman/cron/scheduler_tests.rs index ba00ade40..70ce91a51 100644 --- a/src/openhuman/cron/scheduler_tests.rs +++ b/src/openhuman/cron/scheduler_tests.rs @@ -1,8 +1,9 @@ use super::*; use crate::openhuman::config::Config; -use crate::openhuman::cron::{self, DeliveryConfig}; +use crate::openhuman::cron::{self, ActiveHours, DeliveryConfig}; use crate::openhuman::security::SecurityPolicy; -use chrono::{Duration as ChronoDuration, Utc}; +use chrono::{Duration as ChronoDuration, Timelike, Utc}; +use std::sync::Arc; use tempfile::TempDir; async fn test_config(tmp: &TempDir) -> Config { @@ -24,6 +25,7 @@ fn test_job(command: &str) -> CronJob { schedule: crate::openhuman::cron::Schedule::Cron { expr: "* * * * *".into(), tz: None, + active_hours: None, }, command: command.into(), prompt: None, @@ -221,6 +223,62 @@ async fn persist_job_result_records_run_and_reschedules_shell_job() { assert_eq!(updated.last_status.as_deref(), Some("ok")); } +#[tokio::test] +async fn scheduler_flow_runs_active_hours_job_and_reschedules_inside_window() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp).await; + let active_minute = Utc::now() + ChronoDuration::minutes(2); + let active_hm = format!("{:02}:{:02}", active_minute.hour(), active_minute.minute()); + let active_hours = ActiveHours { + start: active_hm.clone(), + end: active_hm.clone(), + }; + let mut job = cron::add_shell_job( + &config, + Some("active-hours-e2e".into()), + Schedule::Cron { + expr: "* * * * *".into(), + tz: Some("UTC".into()), + active_hours: Some(active_hours.clone()), + }, + "echo active-hours-fired", + ) + .unwrap(); + job.next_run = Utc::now() - ChronoDuration::seconds(1); + + let security = Arc::new(SecurityPolicy::from_config( + &config.autonomy, + &config.workspace_dir, + )); + process_due_jobs(&config, &security, vec![job.clone()]).await; + + let stored = cron::get_job(&config, &job.id).unwrap(); + assert_eq!(stored.last_status.as_deref(), Some("ok")); + assert!(stored + .last_output + .as_deref() + .unwrap_or_default() + .contains("active-hours-fired")); + assert_eq!( + stored.schedule, + Schedule::Cron { + expr: "* * * * *".into(), + tz: Some("UTC".into()), + active_hours: Some(active_hours), + } + ); + + let next_hm = format!( + "{:02}:{:02}", + stored.next_run.hour(), + stored.next_run.minute() + ); + assert_eq!(next_hm, active_hm); + let runs = cron::list_runs(&config, &job.id, 10).unwrap(); + assert_eq!(runs.len(), 1); + assert_eq!(runs[0].status, "ok"); +} + #[tokio::test] async fn persist_job_result_success_deletes_one_shot() { let tmp = TempDir::new().unwrap(); @@ -353,6 +411,7 @@ fn is_one_shot_auto_delete_false_for_cron_schedule() { job.schedule = Schedule::Cron { expr: "0 * * * *".into(), tz: None, + active_hours: None, }; assert!(!is_one_shot_auto_delete(&job)); } diff --git a/src/openhuman/cron/seed.rs b/src/openhuman/cron/seed.rs index 4d8dbb263..8f1ad6438 100644 --- a/src/openhuman/cron/seed.rs +++ b/src/openhuman/cron/seed.rs @@ -104,14 +104,15 @@ fn prune_legacy_welcome(config: &Config, existing: &[crate::openhuman::cron::Cro } } -/// Daily morning briefing at 7:00 AM UTC. -/// +/// Daily morning briefing at 7:00 AM in the device-local timezone +/// (unless a timezone is later set explicitly). /// The cron expression `0 7 * * *` fires once per day. Users can later /// adjust the schedule or time zone via `cron.update_job`. fn seed_morning_briefing(config: &Config) -> Result<()> { let schedule = Schedule::Cron { expr: "0 7 * * *".to_string(), tz: None, + active_hours: None, }; let prompt = concat!( diff --git a/src/openhuman/cron/store.rs b/src/openhuman/cron/store.rs index 2394901e4..c66d54db1 100644 --- a/src/openhuman/cron/store.rs +++ b/src/openhuman/cron/store.rs @@ -15,6 +15,7 @@ pub fn add_job(config: &Config, expression: &str, command: &str) -> Result, expression: &str) -> Result, + #[serde(default)] + active_hours: Option, }, At { at: DateTime, @@ -214,6 +222,7 @@ mod tests { let s = Schedule::Cron { expr: "0 9 * * *".into(), tz: Some("America/Los_Angeles".into()), + active_hours: None, }; let v = serde_json::to_value(&s).unwrap(); assert_eq!(v["kind"], "cron"); @@ -232,10 +241,28 @@ mod tests { Schedule::Cron { expr: "*/5 * * * *".into(), tz: None, + active_hours: None, } ); } + #[test] + fn schedule_cron_variant_roundtrips_with_active_hours() { + let s = Schedule::Cron { + expr: "*/15 * * * *".into(), + tz: Some("UTC".into()), + active_hours: Some(ActiveHours { + start: "09:00".into(), + end: "17:30".into(), + }), + }; + let v = serde_json::to_value(&s).unwrap(); + assert_eq!(v["active_hours"]["start"], "09:00"); + assert_eq!(v["active_hours"]["end"], "17:30"); + let back: Schedule = serde_json::from_value(v).unwrap(); + assert_eq!(back, s); + } + #[test] fn schedule_at_variant_roundtrips_with_utc_timestamp() { let at = Utc.with_ymd_and_hms(2027, 1, 15, 12, 0, 0).unwrap(); diff --git a/src/openhuman/tools/impl/cron/add.rs b/src/openhuman/tools/impl/cron/add.rs index e275d2ce0..4a8d45d5e 100644 --- a/src/openhuman/tools/impl/cron/add.rs +++ b/src/openhuman/tools/impl/cron/add.rs @@ -24,7 +24,7 @@ impl Tool for CronAddTool { } fn description(&self) -> &str { - "Create a scheduled cron job (shell or agent) with cron/at/every schedules" + "Create a scheduled cron job (shell or agent) with cron/at/every schedules. Standardizes on device-local timezone unless 'tz' is set. Note: the scheduler polls on an interval (default 15s, minimum 5s) and does not 'catch up' missed runs." } fn parameters_schema(&self) -> serde_json::Value { @@ -33,8 +33,50 @@ impl Tool for CronAddTool { "properties": { "name": { "type": "string", "description": "Short human-readable name for the job (e.g. 'drink_water_reminder'). Always provide a name." }, "schedule": { - "type": "object", - "description": "Schedule object: {kind:'cron',expr,tz?} | {kind:'at',at} | {kind:'every',every_ms}" + "description": "Schedule: cron expression, one-shot at time, or fixed interval.", + "oneOf": [ + { + "type": "object", + "description": "Repeating cron schedule. 'tz' is an IANA timezone (e.g. 'America/Los_Angeles'); defaults to device-local timezone.", + "properties": { + "kind": { "type": "string", "const": "cron" }, + "expr": { "type": "string", "description": "Cron expression (5, 6, or 7 fields)" }, + "tz": { "type": "string", "description": "Optional IANA timezone name" }, + "active_hours": { + "type": "object", + "description": "Optional: only run during these local hours", + "properties": { + "start": { "type": "string", "description": "Start time HH:MM (e.g. '09:00')" }, + "end": { "type": "string", "description": "End time HH:MM (e.g. '17:00')" } + }, + "required": ["start", "end"], + "additionalProperties": false + } + }, + "required": ["kind", "expr"], + "additionalProperties": false + }, + { + "type": "object", + "description": "One-shot job that runs once at a specific UTC instant.", + "properties": { + "kind": { "type": "string", "const": "at" }, + "at": { "type": "string", "description": "ISO-8601 UTC timestamp" } + }, + "required": ["kind", "at"], + "additionalProperties": false + }, + { + "type": "object", + "description": "Repeating job that fires every N milliseconds.", + "properties": { + "kind": { "type": "string", "const": "every" }, + "every_ms": { "type": "integer", "description": "Interval in milliseconds (must be > 0)" } + }, + "required": ["kind", "every_ms"], + "additionalProperties": false + } + ] }, "job_type": { "type": "string", "enum": ["shell", "agent"] }, "command": { "type": "string" }, @@ -214,6 +256,7 @@ impl Tool for CronAddTool { mod tests { use super::*; use crate::openhuman::config::Config; + use crate::openhuman::cron::ActiveHours; use crate::openhuman::security::AutonomyLevel; use tempfile::TempDir; @@ -254,6 +297,46 @@ mod tests { assert!(result.output().contains("next_run")); } + #[tokio::test] + async fn adds_active_hours_shell_job_from_tool_payload() { + let tmp = TempDir::new().unwrap(); + let cfg = test_config(&tmp).await; + let tool = CronAddTool::new(cfg.clone(), test_security(&cfg)); + let result = tool + .execute(json!({ + "name": "work_hours_ping", + "schedule": { + "kind": "cron", + "expr": "* * * * *", + "tz": "UTC", + "active_hours": { + "start": "09:00", + "end": "17:00" + } + }, + "job_type": "shell", + "command": "echo ok" + })) + .await + .unwrap(); + + assert!(!result.is_error, "{:?}", result.output()); + let jobs = cron::list_jobs(&cfg).unwrap(); + assert_eq!(jobs.len(), 1); + assert_eq!(jobs[0].name.as_deref(), Some("work_hours_ping")); + assert_eq!( + jobs[0].schedule, + Schedule::Cron { + expr: "* * * * *".into(), + tz: Some("UTC".into()), + active_hours: Some(ActiveHours { + start: "09:00".into(), + end: "17:00".into(), + }), + } + ); + } + #[tokio::test] async fn blocks_disallowed_shell_command() { let tmp = TempDir::new().unwrap(); diff --git a/src/openhuman/tools/impl/system/schedule.rs b/src/openhuman/tools/impl/system/schedule.rs index 4203b7f6b..bee5cdd4e 100644 --- a/src/openhuman/tools/impl/system/schedule.rs +++ b/src/openhuman/tools/impl/system/schedule.rs @@ -278,6 +278,7 @@ impl ScheduleTool { Schedule::Cron { expr: expr.to_string(), tz: None, + active_hours: None, } } else if let Some(delay_str) = delay { let at = Utc::now() + cron::parse_human_delay(delay_str)?;