mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
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>
This commit is contained in:
co-authored by
google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
Steven Enamakel
parent
23a18c9c3f
commit
eff8fe00c6
@@ -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,
|
||||
};
|
||||
|
||||
@@ -45,6 +45,10 @@ pub fn resume_job(config: &Config, id: &str) -> Result<CronJob> {
|
||||
}
|
||||
|
||||
/// 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
|
||||
|
||||
@@ -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),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
+253
-20
@@ -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<Utc>) -> Result<DateTime<Utc>> {
|
||||
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<Utc>) -> Result
|
||||
|
||||
pub fn validate_schedule(schedule: &Schedule, now: DateTime<Utc>) -> 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<String> {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy)]
|
||||
enum ScheduleTimeZone {
|
||||
Local,
|
||||
Named(chrono_tz::Tz),
|
||||
}
|
||||
|
||||
impl ScheduleTimeZone {
|
||||
fn parse(tz: Option<&str>) -> Result<Self> {
|
||||
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<Utc>,
|
||||
expr: &str,
|
||||
) -> Result<DateTime<Utc>> {
|
||||
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<Utc>) -> 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<Self> {
|
||||
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<String> {
|
||||
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"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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));
|
||||
}
|
||||
|
||||
@@ -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!(
|
||||
|
||||
@@ -15,6 +15,7 @@ pub fn add_job(config: &Config, expression: &str, command: &str) -> Result<CronJ
|
||||
let schedule = Schedule::Cron {
|
||||
expr: expression.to_string(),
|
||||
tz: None,
|
||||
active_hours: None,
|
||||
};
|
||||
add_shell_job(config, None, schedule, command)
|
||||
}
|
||||
@@ -495,6 +496,7 @@ fn decode_schedule(schedule_raw: Option<&str>, expression: &str) -> Result<Sched
|
||||
Ok(Schedule::Cron {
|
||||
expr: expression.to_string(),
|
||||
tz: None,
|
||||
active_hours: None,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use super::*;
|
||||
use crate::openhuman::config::Config;
|
||||
use crate::openhuman::cron::ActiveHours;
|
||||
use chrono::Duration as ChronoDuration;
|
||||
use tempfile::TempDir;
|
||||
|
||||
@@ -24,6 +25,39 @@ fn add_job_accepts_five_field_expression() {
|
||||
assert!(matches!(job.schedule, Schedule::Cron { .. }));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn add_shell_job_persists_active_hours_schedule() {
|
||||
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,
|
||||
Some("business-hours".into()),
|
||||
Schedule::Cron {
|
||||
expr: "0 9 * * *".into(),
|
||||
tz: Some("UTC".into()),
|
||||
active_hours: Some(active_hours.clone()),
|
||||
},
|
||||
"echo ok",
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let stored = get_job(&config, &job.id).unwrap();
|
||||
assert_eq!(stored.expression, "0 9 * * *");
|
||||
assert_eq!(
|
||||
stored.schedule,
|
||||
Schedule::Cron {
|
||||
expr: "0 9 * * *".into(),
|
||||
tz: Some("UTC".into()),
|
||||
active_hours: Some(active_hours),
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn add_list_remove_roundtrip() {
|
||||
let tmp = TempDir::new().unwrap();
|
||||
|
||||
@@ -51,6 +51,12 @@ impl SessionTarget {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
|
||||
pub struct ActiveHours {
|
||||
pub start: String,
|
||||
pub end: String,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
|
||||
#[serde(tag = "kind", rename_all = "lowercase")]
|
||||
pub enum Schedule {
|
||||
@@ -58,6 +64,8 @@ pub enum Schedule {
|
||||
expr: String,
|
||||
#[serde(default)]
|
||||
tz: Option<String>,
|
||||
#[serde(default)]
|
||||
active_hours: Option<ActiveHours>,
|
||||
},
|
||||
At {
|
||||
at: DateTime<Utc>,
|
||||
@@ -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();
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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)?;
|
||||
|
||||
Reference in New Issue
Block a user