From 561c3ccbada97068846eae3e032193d78a1dd3c9 Mon Sep 17 00:00:00 2001 From: 2witstudios <2witstudios@gmail.com> Date: Tue, 10 Mar 2026 15:59:35 -0500 Subject: [PATCH 1/3] refactor: Extract schedule_def tests to submodule Move 442 lines of tests from schedule_def.rs to schedule_def/tests.rs, following the same pattern as engine/, protocol/, types/, and output/. Before: 778 LoC (57% tests) After: 334 LoC production code + 442 LoC tests Co-Authored-By: Claude Opus 4.6 --- crates/pu-core/src/schedule_def.rs | 777 ----------------------- crates/pu-core/src/schedule_def/mod.rs | 334 ++++++++++ crates/pu-core/src/schedule_def/tests.rs | 442 +++++++++++++ 3 files changed, 776 insertions(+), 777 deletions(-) delete mode 100644 crates/pu-core/src/schedule_def.rs create mode 100644 crates/pu-core/src/schedule_def/mod.rs create mode 100644 crates/pu-core/src/schedule_def/tests.rs diff --git a/crates/pu-core/src/schedule_def.rs b/crates/pu-core/src/schedule_def.rs deleted file mode 100644 index e70658a..0000000 --- a/crates/pu-core/src/schedule_def.rs +++ /dev/null @@ -1,777 +0,0 @@ -use std::collections::HashMap; -use std::path::Path; - -use chrono::{DateTime, Datelike, Duration, NaiveDate, TimeZone, Timelike, Utc, Weekday}; -use serde::{Deserialize, Serialize}; - -use crate::paths; - -#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] -#[serde(rename_all = "snake_case")] -pub enum Recurrence { - #[default] - None, - Hourly, - Daily, - Weekdays, - Weekly, - Monthly, -} - -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -#[serde(tag = "type", rename_all = "snake_case")] -pub enum ScheduleTrigger { - AgentDef { - name: String, - }, - SwarmDef { - name: String, - #[serde(default)] - vars: HashMap, - }, - InlinePrompt { - prompt: String, - #[serde(default = "default_agent")] - agent: String, - }, -} - -fn default_agent() -> String { - "claude".to_string() -} - -fn default_enabled() -> bool { - true -} - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct ScheduleDef { - pub name: String, - #[serde(default = "default_enabled")] - pub enabled: bool, - #[serde(default)] - pub recurrence: Recurrence, - pub start_at: DateTime, - #[serde(default)] - pub next_run: Option>, - pub trigger: ScheduleTrigger, - pub project_root: String, - #[serde(default)] - pub target: String, - /// Whether the scheduled agent spawns in the project root (true) or a worktree (false) - #[serde(default = "crate::serde_defaults::default_true")] - pub root: bool, - /// Worktree/branch name when `root` is false - #[serde(default)] - pub agent_name: Option, - /// "local" or "global" — set at load time, not serialized - #[serde(skip)] - pub scope: String, - pub created_at: DateTime, -} - -impl ScheduleDef { - /// Validate that `root` and `agent_name` are consistent: - /// - root=true → agent_name must be None - /// - root=false → agent_name must be Some(non-empty) - pub fn validate(&self) -> Result<(), std::io::Error> { - if self.root { - if self.agent_name.is_some() { - return Err(std::io::Error::new( - std::io::ErrorKind::InvalidInput, - "agent_name must not be set when root is true", - )); - } - } else if self.agent_name.as_ref().is_none_or(String::is_empty) { - return Err(std::io::Error::new( - std::io::ErrorKind::InvalidInput, - "agent_name is required when root is false", - )); - } - Ok(()) - } -} - -/// Scan both local and global schedule definition directories. Local defs take priority. -pub fn list_schedule_defs(project_root: &Path) -> Vec { - let mut seen = HashMap::new(); - let mut result = Vec::new(); - - let local_dir = paths::schedules_dir(project_root); - if local_dir.is_dir() { - for def in scan_dir(&local_dir, "local") { - seen.insert(def.name.clone(), result.len()); - result.push(def); - } - } - - if let Ok(global_dir) = paths::global_schedules_dir() { - if global_dir.is_dir() { - for def in scan_dir(&global_dir, "global") { - if !seen.contains_key(&def.name) { - result.push(def); - } - } - } - } - - result -} - -/// Find a schedule definition by name. Checks local first, then global. -pub fn find_schedule_def(project_root: &Path, name: &str) -> Option { - let local_dir = paths::schedules_dir(project_root); - if local_dir.is_dir() { - if let Some(def) = find_in_dir(&local_dir, name, "local") { - return Some(def); - } - } - if let Ok(global_dir) = paths::global_schedules_dir() { - if global_dir.is_dir() { - if let Some(def) = find_in_dir(&global_dir, name, "global") { - return Some(def); - } - } - } - None -} - -/// Save a schedule definition as a YAML file. Creates the directory if needed. -pub fn save_schedule_def(dir: &Path, def: &ScheduleDef) -> Result<(), std::io::Error> { - crate::validation::validate_name(&def.name)?; - def.validate()?; - if def.project_root.is_empty() { - return Err(std::io::Error::new( - std::io::ErrorKind::InvalidInput, - "project_root must not be empty", - )); - } - std::fs::create_dir_all(dir)?; - let path = dir.join(format!("{}.yaml", def.name)); - let yaml = serde_yml::to_string(def).map_err(std::io::Error::other)?; - std::fs::write(path, yaml) -} - -/// Delete a schedule definition file. Returns true if the file existed. -pub fn delete_schedule_def(dir: &Path, name: &str) -> Result { - crate::validation::validate_name(name)?; - let path = dir.join(format!("{name}.yaml")); - if path.is_file() { - std::fs::remove_file(path)?; - Ok(true) - } else { - Ok(false) - } -} - -/// Compute the next occurrence of a recurring schedule after `after`. -/// Returns None if the schedule is one-shot and `after` >= `base`. -pub fn next_occurrence( - base: DateTime, - recurrence: &Recurrence, - after: DateTime, -) -> Option> { - // Clamp: never return an occurrence before start_at (base) - let after = if after < base { - base - Duration::seconds(1) - } else { - after - }; - match recurrence { - Recurrence::None => next_none_occurrence(base, after), - Recurrence::Hourly => next_hourly_occurrence(base, after), - Recurrence::Daily => next_daily_occurrence(base, after), - Recurrence::Weekdays => next_weekdays_occurrence(base, after), - Recurrence::Weekly => next_weekly_occurrence(base, after), - Recurrence::Monthly => next_monthly_occurrence(base, after), - } -} - -/// Build a NaiveDateTime on `after`'s date at `base`'s hour/minute/second. -/// Shared by Daily, Weekdays, and Weekly recurrence calculations. -fn naive_at_base_time(base: DateTime, after: DateTime) -> chrono::NaiveDateTime { - after - .date_naive() - .and_hms_opt(base.hour(), base.minute(), base.second()) - .unwrap() -} - -fn next_none_occurrence(base: DateTime, after: DateTime) -> Option> { - if after <= base { Some(base) } else { None } -} - -fn next_hourly_occurrence(base: DateTime, after: DateTime) -> Option> { - // Next occurrence at base's minute, after `after` - let mut candidate = after - .with_minute(base.minute()) - .unwrap() - .with_second(base.second()) - .unwrap() - .with_nanosecond(0) - .unwrap(); - if candidate <= after { - candidate += Duration::hours(1); - } - Some(candidate) -} - -fn next_daily_occurrence(base: DateTime, after: DateTime) -> Option> { - let mut candidate = naive_at_base_time(base, after); - if Utc.from_utc_datetime(&candidate) <= after { - candidate += Duration::days(1); - } - Some(Utc.from_utc_datetime(&candidate)) -} - -fn next_weekdays_occurrence(base: DateTime, after: DateTime) -> Option> { - let mut candidate = naive_at_base_time(base, after); - if Utc.from_utc_datetime(&candidate) <= after { - candidate += Duration::days(1); - } - // Skip weekends - loop { - let wd = candidate.weekday(); - if wd != Weekday::Sat && wd != Weekday::Sun { - break; - } - candidate += Duration::days(1); - } - Some(Utc.from_utc_datetime(&candidate)) -} - -fn next_weekly_occurrence(base: DateTime, after: DateTime) -> Option> { - let mut candidate = naive_at_base_time(base, after); - // Align to same weekday as base - let target_weekday = base.weekday(); - let current_weekday = candidate.weekday(); - let days_ahead = (target_weekday.num_days_from_monday() as i64 - - current_weekday.num_days_from_monday() as i64 - + 7) - % 7; - candidate += Duration::days(days_ahead); - if Utc.from_utc_datetime(&candidate) <= after { - candidate += Duration::weeks(1); - } - Some(Utc.from_utc_datetime(&candidate)) -} - -fn next_monthly_occurrence(base: DateTime, after: DateTime) -> Option> { - let target_day = base.day(); - let target_time = base.time(); - let mut year = after.year(); - let mut month = after.month(); - - // Start from after's month - loop { - if let Some(date) = NaiveDate::from_ymd_opt(year, month, target_day) { - let candidate = Utc.from_utc_datetime(&date.and_time(target_time)); - if candidate > after { - return Some(candidate); - } - } - // Advance month - month += 1; - if month > 12 { - month = 1; - year += 1; - } - // Safety: don't loop forever (covers 4 years = 48 months max) - if year > after.year() + 4 { - break; - } - } - None -} - -fn scan_dir(dir: &Path, scope: &str) -> Vec { - let mut defs = Vec::new(); - let Ok(entries) = std::fs::read_dir(dir) else { - return defs; - }; - for entry in entries.flatten() { - let path = entry.path(); - if path.extension().and_then(|e| e.to_str()) == Some("yaml") { - if let Ok(content) = std::fs::read_to_string(&path) { - match serde_yml::from_str::(&content) { - Ok(mut def) => { - if let Err(e) = def.validate() { - eprintln!("warning: invalid schedule {}: {e}", path.display()); - continue; - } - def.scope = scope.to_string(); - defs.push(def); - } - Err(e) => { - eprintln!("warning: failed to parse {}: {e}", path.display()); - } - } - } - } - } - defs.sort_by(|a, b| a.name.cmp(&b.name)); - defs -} - -fn find_in_dir(dir: &Path, name: &str, scope: &str) -> Option { - let path = dir.join(format!("{name}.yaml")); - if path.is_file() { - if let Ok(content) = std::fs::read_to_string(&path) { - if let Ok(mut def) = serde_yml::from_str::(&content) { - if def.validate().is_err() { - return None; - } - def.scope = scope.to_string(); - return Some(def); - } - } - } - scan_dir(dir, scope) - .into_iter() - .find(|def| def.name == name) -} - -#[cfg(test)] -mod tests { - use super::*; - use tempfile::TempDir; - - fn isolate_home(tmp: &TempDir) { - paths::set_home_override(Some(tmp.path().to_path_buf())); - } - - fn make_trigger() -> ScheduleTrigger { - ScheduleTrigger::AgentDef { - name: "security-review".to_string(), - } - } - - fn make_schedule_def(name: &str) -> ScheduleDef { - ScheduleDef { - name: name.to_string(), - enabled: true, - recurrence: Recurrence::Daily, - start_at: Utc::now(), - next_run: None, - trigger: make_trigger(), - project_root: "/projects/myapp".to_string(), - target: String::new(), - root: true, - agent_name: None, - scope: String::new(), - created_at: Utc::now(), - } - } - - // --- Deserialization (REQ-SCHED-001) --- - - #[test] - fn given_schedule_def_yaml_should_deserialize() { - let yaml = r#" -name: nightly-review -enabled: true -recurrence: daily -start_at: "2025-01-01T03:00:00Z" -trigger: - type: agent_def - name: security-review -project_root: /projects/myapp -created_at: "2025-01-01T00:00:00Z" -"#; - let def: ScheduleDef = serde_yml::from_str(yaml).unwrap(); - assert_eq!(def.name, "nightly-review"); - assert!(def.enabled); - assert_eq!(def.recurrence, Recurrence::Daily); - assert!( - matches!(def.trigger, ScheduleTrigger::AgentDef { ref name } if name == "security-review") - ); - assert_eq!(def.project_root, "/projects/myapp"); - } - - #[test] - fn given_minimal_schedule_yaml_should_use_defaults() { - let yaml = r#" -name: quick -start_at: "2025-06-01T12:00:00Z" -trigger: - type: agent_def - name: test -project_root: /tmp -created_at: "2025-06-01T00:00:00Z" -"#; - let def: ScheduleDef = serde_yml::from_str(yaml).unwrap(); - assert!(def.enabled); // default true - assert_eq!(def.recurrence, Recurrence::None); // default none - assert_eq!(def.target, ""); // default empty - assert!(def.next_run.is_none()); // default none - assert!(def.root); // default true (backward compat) - assert!(def.agent_name.is_none()); // default none - } - - #[test] - fn given_schedule_with_worktree_fields_should_round_trip() { - let yaml = r#" -name: overnight-build -start_at: "2025-06-01T22:30:00Z" -trigger: - type: inline_prompt - prompt: "build a feature" -project_root: /projects/myapp -root: false -agent_name: overnight-build -created_at: "2025-06-01T00:00:00Z" -"#; - let def: ScheduleDef = serde_yml::from_str(yaml).unwrap(); - assert!(!def.root); - assert_eq!(def.agent_name.as_deref(), Some("overnight-build")); - - // Round-trip through YAML - let serialized = serde_yml::to_string(&def).unwrap(); - let reparsed: ScheduleDef = serde_yml::from_str(&serialized).unwrap(); - assert!(!reparsed.root); - assert_eq!(reparsed.agent_name.as_deref(), Some("overnight-build")); - } - - // --- Validation --- - - #[test] - fn given_root_true_with_no_agent_name_should_validate() { - let def = make_schedule_def("test"); - assert!(def.validate().is_ok()); - } - - #[test] - fn given_root_true_with_agent_name_should_reject() { - let mut def = make_schedule_def("test"); - def.agent_name = Some("bad".to_string()); - assert!(def.validate().is_err()); - } - - #[test] - fn given_root_true_with_empty_agent_name_should_reject() { - let mut def = make_schedule_def("test"); - def.agent_name = Some(String::new()); - assert!(def.validate().is_err()); - } - - #[test] - fn given_root_false_with_agent_name_should_validate() { - let mut def = make_schedule_def("test"); - def.root = false; - def.agent_name = Some("my-worktree".to_string()); - assert!(def.validate().is_ok()); - } - - #[test] - fn given_root_false_with_no_agent_name_should_reject() { - let mut def = make_schedule_def("test"); - def.root = false; - assert!(def.validate().is_err()); - } - - #[test] - fn given_root_false_with_empty_agent_name_should_reject() { - let mut def = make_schedule_def("test"); - def.root = false; - def.agent_name = Some(String::new()); - assert!(def.validate().is_err()); - } - - #[test] - fn given_trigger_agent_def_should_round_trip() { - let trigger = ScheduleTrigger::AgentDef { - name: "reviewer".to_string(), - }; - let yaml = serde_yml::to_string(&trigger).unwrap(); - let parsed: ScheduleTrigger = serde_yml::from_str(&yaml).unwrap(); - assert_eq!(parsed, trigger); - } - - #[test] - fn given_trigger_swarm_def_with_vars_should_round_trip() { - let mut vars = HashMap::new(); - vars.insert("branch".to_string(), "main".to_string()); - let trigger = ScheduleTrigger::SwarmDef { - name: "full-stack".to_string(), - vars, - }; - let yaml = serde_yml::to_string(&trigger).unwrap(); - let parsed: ScheduleTrigger = serde_yml::from_str(&yaml).unwrap(); - assert_eq!(parsed, trigger); - } - - #[test] - fn given_trigger_inline_prompt_should_round_trip() { - let trigger = ScheduleTrigger::InlinePrompt { - prompt: "Review all deps".to_string(), - agent: "claude".to_string(), - }; - let yaml = serde_yml::to_string(&trigger).unwrap(); - let parsed: ScheduleTrigger = serde_yml::from_str(&yaml).unwrap(); - assert_eq!(parsed, trigger); - } - - // --- CRUD (REQ-SCHED-002 through REQ-SCHED-006) --- - - #[test] - fn given_local_and_global_schedule_defs_should_list_local_first() { - let tmp = TempDir::new().unwrap(); - isolate_home(&tmp); - let root = tmp.path(); - let local_dir = paths::schedules_dir(root); - std::fs::create_dir_all(&local_dir).unwrap(); - - let mut def = make_schedule_def("nightly"); - save_schedule_def(&local_dir, &def).unwrap(); - def.name = "weekly".to_string(); - save_schedule_def(&local_dir, &def).unwrap(); - - let defs = list_schedule_defs(root); - assert_eq!(defs.len(), 2); - assert_eq!(defs[0].name, "nightly"); - assert_eq!(defs[1].name, "weekly"); - assert_eq!(defs[0].scope, "local"); - } - - #[test] - fn given_schedule_def_name_should_find_by_name() { - let tmp = TempDir::new().unwrap(); - let root = tmp.path(); - let local_dir = paths::schedules_dir(root); - std::fs::create_dir_all(&local_dir).unwrap(); - - let def = make_schedule_def("nightly"); - save_schedule_def(&local_dir, &def).unwrap(); - - let found = find_schedule_def(root, "nightly"); - assert!(found.is_some()); - assert_eq!(found.unwrap().name, "nightly"); - } - - #[test] - fn given_no_schedule_defs_should_return_empty_list() { - let tmp = TempDir::new().unwrap(); - isolate_home(&tmp); - let defs = list_schedule_defs(tmp.path()); - assert!(defs.is_empty()); - } - - #[test] - fn given_schedule_def_should_save_and_load() { - let tmp = TempDir::new().unwrap(); - let dir = tmp.path().join("schedules"); - let def = make_schedule_def("test-schedule"); - save_schedule_def(&dir, &def).unwrap(); - - let path = dir.join("test-schedule.yaml"); - assert!(path.is_file()); - - let content = std::fs::read_to_string(&path).unwrap(); - let loaded: ScheduleDef = serde_yml::from_str(&content).unwrap(); - assert_eq!(loaded.name, "test-schedule"); - assert_eq!(loaded.recurrence, Recurrence::Daily); - } - - #[test] - fn given_invalid_name_should_reject() { - let tmp = TempDir::new().unwrap(); - let dir = tmp.path().join("schedules"); - let mut def = make_schedule_def("../evil"); - def.name = "../evil".to_string(); - assert!(save_schedule_def(&dir, &def).is_err()); - } - - #[test] - fn given_existing_schedule_def_should_delete_and_return_true() { - let tmp = TempDir::new().unwrap(); - let dir = tmp.path().join("schedules"); - std::fs::create_dir_all(&dir).unwrap(); - let def = make_schedule_def("nightly"); - save_schedule_def(&dir, &def).unwrap(); - - let deleted = delete_schedule_def(&dir, "nightly").unwrap(); - assert!(deleted); - assert!(!dir.join("nightly.yaml").exists()); - } - - #[test] - fn given_nonexistent_schedule_def_should_return_false() { - let tmp = TempDir::new().unwrap(); - let dir = tmp.path().join("schedules"); - std::fs::create_dir_all(&dir).unwrap(); - - let deleted = delete_schedule_def(&dir, "nonexistent").unwrap(); - assert!(!deleted); - } - - #[test] - fn given_duplicate_name_in_local_and_global_should_prefer_local() { - let tmp = TempDir::new().unwrap(); - let root = tmp.path(); - let local_dir = paths::schedules_dir(root); - std::fs::create_dir_all(&local_dir).unwrap(); - - let def = make_schedule_def("nightly"); - save_schedule_def(&local_dir, &def).unwrap(); - - let found = find_schedule_def(root, "nightly").unwrap(); - assert_eq!(found.scope, "local"); - } - - #[test] - fn given_empty_project_root_should_reject() { - let tmp = TempDir::new().unwrap(); - let dir = tmp.path().join("schedules"); - let mut def = make_schedule_def("test"); - def.project_root = String::new(); - assert!(save_schedule_def(&dir, &def).is_err()); - } - - // --- Recurrence calculator (REQ-SCHED-010 through REQ-SCHED-018) --- - - fn utc(y: i32, m: u32, d: u32, h: u32, min: u32, s: u32) -> DateTime { - Utc.with_ymd_and_hms(y, m, d, h, min, s).unwrap() - } - - #[test] - fn given_none_recurrence_before_base_should_return_base() { - let base = utc(2025, 6, 15, 10, 0, 0); - let after = utc(2025, 6, 14, 10, 0, 0); - assert_eq!(next_occurrence(base, &Recurrence::None, after), Some(base)); - } - - #[test] - fn given_none_recurrence_after_base_should_return_none() { - let base = utc(2025, 6, 15, 10, 0, 0); - let after = utc(2025, 6, 16, 10, 0, 0); - assert_eq!(next_occurrence(base, &Recurrence::None, after), None); - } - - #[test] - fn given_hourly_recurrence_should_return_next_hour() { - let base = utc(2025, 6, 15, 10, 30, 0); - let after = utc(2025, 6, 15, 11, 0, 0); - let next = next_occurrence(base, &Recurrence::Hourly, after).unwrap(); - assert_eq!(next, utc(2025, 6, 15, 11, 30, 0)); - } - - #[test] - fn given_hourly_after_same_minute_should_advance_one_hour() { - let base = utc(2025, 6, 15, 10, 30, 0); - let after = utc(2025, 6, 15, 10, 30, 0); - let next = next_occurrence(base, &Recurrence::Hourly, after).unwrap(); - assert_eq!(next, utc(2025, 6, 15, 11, 30, 0)); - } - - #[test] - fn given_daily_recurrence_should_return_next_day() { - let base = utc(2025, 6, 15, 3, 0, 0); - let after = utc(2025, 6, 15, 4, 0, 0); - let next = next_occurrence(base, &Recurrence::Daily, after).unwrap(); - assert_eq!(next, utc(2025, 6, 16, 3, 0, 0)); - } - - #[test] - fn given_daily_before_time_today_should_return_today() { - let base = utc(2025, 6, 15, 15, 0, 0); - let after = utc(2025, 6, 15, 10, 0, 0); - let next = next_occurrence(base, &Recurrence::Daily, after).unwrap(); - assert_eq!(next, utc(2025, 6, 15, 15, 0, 0)); - } - - #[test] - fn given_weekdays_on_friday_should_skip_to_monday() { - // 2025-06-13 is a Friday - let base = utc(2025, 6, 13, 9, 0, 0); - let after = utc(2025, 6, 13, 10, 0, 0); - let next = next_occurrence(base, &Recurrence::Weekdays, after).unwrap(); - // Should skip to Monday 2025-06-16 - assert_eq!(next, utc(2025, 6, 16, 9, 0, 0)); - assert_eq!(next.weekday(), Weekday::Mon); - } - - #[test] - fn given_weekdays_on_saturday_should_skip_to_monday() { - // 2025-06-14 is a Saturday - let base = utc(2025, 6, 14, 9, 0, 0); - let after = utc(2025, 6, 14, 0, 0, 0); - let next = next_occurrence(base, &Recurrence::Weekdays, after).unwrap(); - assert_eq!(next, utc(2025, 6, 16, 9, 0, 0)); - assert_eq!(next.weekday(), Weekday::Mon); - } - - #[test] - fn given_weekdays_on_sunday_should_skip_to_monday() { - // 2025-06-15 is a Sunday - let base = utc(2025, 6, 15, 9, 0, 0); - let after = utc(2025, 6, 15, 0, 0, 0); - let next = next_occurrence(base, &Recurrence::Weekdays, after).unwrap(); - assert_eq!(next, utc(2025, 6, 16, 9, 0, 0)); - assert_eq!(next.weekday(), Weekday::Mon); - } - - #[test] - fn given_weekdays_on_wednesday_should_return_thursday() { - // 2025-06-11 is a Wednesday - let base = utc(2025, 6, 11, 9, 0, 0); - let after = utc(2025, 6, 11, 10, 0, 0); - let next = next_occurrence(base, &Recurrence::Weekdays, after).unwrap(); - assert_eq!(next, utc(2025, 6, 12, 9, 0, 0)); - assert_eq!(next.weekday(), Weekday::Thu); - } - - #[test] - fn given_weekly_should_return_same_weekday_next_week() { - // 2025-06-11 is a Wednesday - let base = utc(2025, 6, 11, 14, 0, 0); - let after = utc(2025, 6, 11, 15, 0, 0); - let next = next_occurrence(base, &Recurrence::Weekly, after).unwrap(); - assert_eq!(next, utc(2025, 6, 18, 14, 0, 0)); - assert_eq!(next.weekday(), Weekday::Wed); - } - - #[test] - fn given_weekly_same_day_before_time_should_return_same_day() { - // 2025-06-11 is a Wednesday - let base = utc(2025, 6, 11, 14, 0, 0); - let after = utc(2025, 6, 11, 10, 0, 0); - let next = next_occurrence(base, &Recurrence::Weekly, after).unwrap(); - assert_eq!(next, utc(2025, 6, 11, 14, 0, 0)); - } - - #[test] - fn given_monthly_should_return_same_day_next_month() { - let base = utc(2025, 6, 15, 3, 0, 0); - let after = utc(2025, 6, 15, 4, 0, 0); - let next = next_occurrence(base, &Recurrence::Monthly, after).unwrap(); - assert_eq!(next, utc(2025, 7, 15, 3, 0, 0)); - } - - #[test] - fn given_monthly_on_31st_should_skip_short_months() { - let base = utc(2025, 1, 31, 3, 0, 0); - let after = utc(2025, 1, 31, 4, 0, 0); - let next = next_occurrence(base, &Recurrence::Monthly, after).unwrap(); - // Feb has no 31st, March does - assert_eq!(next, utc(2025, 3, 31, 3, 0, 0)); - } - - #[test] - fn given_monthly_on_29th_should_skip_non_leap_feb() { - let base = utc(2025, 1, 29, 3, 0, 0); - let after = utc(2025, 1, 29, 4, 0, 0); - let next = next_occurrence(base, &Recurrence::Monthly, after).unwrap(); - // 2025 is not a leap year, Feb has no 29th - assert_eq!(next, utc(2025, 3, 29, 3, 0, 0)); - } - - #[test] - fn given_daily_with_after_before_start_at_should_not_precede_start_at() { - // start_at is in the future, after is now (before start_at) - let base = utc(2025, 6, 20, 9, 0, 0); - let after = utc(2025, 6, 15, 10, 0, 0); - let next = next_occurrence(base, &Recurrence::Daily, after).unwrap(); - // Should return start_at itself, never a date before it - assert!(next >= base); - assert_eq!(next, utc(2025, 6, 20, 9, 0, 0)); - } -} diff --git a/crates/pu-core/src/schedule_def/mod.rs b/crates/pu-core/src/schedule_def/mod.rs new file mode 100644 index 0000000..90f7bb7 --- /dev/null +++ b/crates/pu-core/src/schedule_def/mod.rs @@ -0,0 +1,334 @@ +use std::collections::HashMap; +use std::path::Path; + +use chrono::{DateTime, Datelike, Duration, NaiveDate, TimeZone, Timelike, Utc, Weekday}; +use serde::{Deserialize, Serialize}; + +use crate::paths; + +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq)] +#[serde(rename_all = "snake_case")] +pub enum Recurrence { + #[default] + None, + Hourly, + Daily, + Weekdays, + Weekly, + Monthly, +} + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(tag = "type", rename_all = "snake_case")] +pub enum ScheduleTrigger { + AgentDef { + name: String, + }, + SwarmDef { + name: String, + #[serde(default)] + vars: HashMap, + }, + InlinePrompt { + prompt: String, + #[serde(default = "default_agent")] + agent: String, + }, +} + +fn default_agent() -> String { + "claude".to_string() +} + +fn default_enabled() -> bool { + true +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ScheduleDef { + pub name: String, + #[serde(default = "default_enabled")] + pub enabled: bool, + #[serde(default)] + pub recurrence: Recurrence, + pub start_at: DateTime, + #[serde(default)] + pub next_run: Option>, + pub trigger: ScheduleTrigger, + pub project_root: String, + #[serde(default)] + pub target: String, + /// Whether the scheduled agent spawns in the project root (true) or a worktree (false) + #[serde(default = "crate::serde_defaults::default_true")] + pub root: bool, + /// Worktree/branch name when `root` is false + #[serde(default)] + pub agent_name: Option, + /// "local" or "global" — set at load time, not serialized + #[serde(skip)] + pub scope: String, + pub created_at: DateTime, +} + +impl ScheduleDef { + /// Validate that `root` and `agent_name` are consistent: + /// - root=true → agent_name must be None + /// - root=false → agent_name must be Some(non-empty) + pub fn validate(&self) -> Result<(), std::io::Error> { + if self.root { + if self.agent_name.is_some() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "agent_name must not be set when root is true", + )); + } + } else if self.agent_name.as_ref().is_none_or(String::is_empty) { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "agent_name is required when root is false", + )); + } + Ok(()) + } +} + +/// Scan both local and global schedule definition directories. Local defs take priority. +pub fn list_schedule_defs(project_root: &Path) -> Vec { + let mut seen = HashMap::new(); + let mut result = Vec::new(); + + let local_dir = paths::schedules_dir(project_root); + if local_dir.is_dir() { + for def in scan_dir(&local_dir, "local") { + seen.insert(def.name.clone(), result.len()); + result.push(def); + } + } + + if let Ok(global_dir) = paths::global_schedules_dir() { + if global_dir.is_dir() { + for def in scan_dir(&global_dir, "global") { + if !seen.contains_key(&def.name) { + result.push(def); + } + } + } + } + + result +} + +/// Find a schedule definition by name. Checks local first, then global. +pub fn find_schedule_def(project_root: &Path, name: &str) -> Option { + let local_dir = paths::schedules_dir(project_root); + if local_dir.is_dir() { + if let Some(def) = find_in_dir(&local_dir, name, "local") { + return Some(def); + } + } + if let Ok(global_dir) = paths::global_schedules_dir() { + if global_dir.is_dir() { + if let Some(def) = find_in_dir(&global_dir, name, "global") { + return Some(def); + } + } + } + None +} + +/// Save a schedule definition as a YAML file. Creates the directory if needed. +pub fn save_schedule_def(dir: &Path, def: &ScheduleDef) -> Result<(), std::io::Error> { + crate::validation::validate_name(&def.name)?; + def.validate()?; + if def.project_root.is_empty() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "project_root must not be empty", + )); + } + std::fs::create_dir_all(dir)?; + let path = dir.join(format!("{}.yaml", def.name)); + let yaml = serde_yml::to_string(def).map_err(std::io::Error::other)?; + std::fs::write(path, yaml) +} + +/// Delete a schedule definition file. Returns true if the file existed. +pub fn delete_schedule_def(dir: &Path, name: &str) -> Result { + crate::validation::validate_name(name)?; + let path = dir.join(format!("{name}.yaml")); + if path.is_file() { + std::fs::remove_file(path)?; + Ok(true) + } else { + Ok(false) + } +} + +/// Compute the next occurrence of a recurring schedule after `after`. +/// Returns None if the schedule is one-shot and `after` >= `base`. +pub fn next_occurrence( + base: DateTime, + recurrence: &Recurrence, + after: DateTime, +) -> Option> { + // Clamp: never return an occurrence before start_at (base) + let after = if after < base { + base - Duration::seconds(1) + } else { + after + }; + match recurrence { + Recurrence::None => next_none_occurrence(base, after), + Recurrence::Hourly => next_hourly_occurrence(base, after), + Recurrence::Daily => next_daily_occurrence(base, after), + Recurrence::Weekdays => next_weekdays_occurrence(base, after), + Recurrence::Weekly => next_weekly_occurrence(base, after), + Recurrence::Monthly => next_monthly_occurrence(base, after), + } +} + +/// Build a NaiveDateTime on `after`'s date at `base`'s hour/minute/second. +/// Shared by Daily, Weekdays, and Weekly recurrence calculations. +fn naive_at_base_time(base: DateTime, after: DateTime) -> chrono::NaiveDateTime { + after + .date_naive() + .and_hms_opt(base.hour(), base.minute(), base.second()) + .unwrap() +} + +fn next_none_occurrence(base: DateTime, after: DateTime) -> Option> { + if after <= base { Some(base) } else { None } +} + +fn next_hourly_occurrence(base: DateTime, after: DateTime) -> Option> { + // Next occurrence at base's minute, after `after` + let mut candidate = after + .with_minute(base.minute()) + .unwrap() + .with_second(base.second()) + .unwrap() + .with_nanosecond(0) + .unwrap(); + if candidate <= after { + candidate += Duration::hours(1); + } + Some(candidate) +} + +fn next_daily_occurrence(base: DateTime, after: DateTime) -> Option> { + let mut candidate = naive_at_base_time(base, after); + if Utc.from_utc_datetime(&candidate) <= after { + candidate += Duration::days(1); + } + Some(Utc.from_utc_datetime(&candidate)) +} + +fn next_weekdays_occurrence(base: DateTime, after: DateTime) -> Option> { + let mut candidate = naive_at_base_time(base, after); + if Utc.from_utc_datetime(&candidate) <= after { + candidate += Duration::days(1); + } + // Skip weekends + loop { + let wd = candidate.weekday(); + if wd != Weekday::Sat && wd != Weekday::Sun { + break; + } + candidate += Duration::days(1); + } + Some(Utc.from_utc_datetime(&candidate)) +} + +fn next_weekly_occurrence(base: DateTime, after: DateTime) -> Option> { + let mut candidate = naive_at_base_time(base, after); + // Align to same weekday as base + let target_weekday = base.weekday(); + let current_weekday = candidate.weekday(); + let days_ahead = (target_weekday.num_days_from_monday() as i64 + - current_weekday.num_days_from_monday() as i64 + + 7) + % 7; + candidate += Duration::days(days_ahead); + if Utc.from_utc_datetime(&candidate) <= after { + candidate += Duration::weeks(1); + } + Some(Utc.from_utc_datetime(&candidate)) +} + +fn next_monthly_occurrence(base: DateTime, after: DateTime) -> Option> { + let target_day = base.day(); + let target_time = base.time(); + let mut year = after.year(); + let mut month = after.month(); + + // Start from after's month + loop { + if let Some(date) = NaiveDate::from_ymd_opt(year, month, target_day) { + let candidate = Utc.from_utc_datetime(&date.and_time(target_time)); + if candidate > after { + return Some(candidate); + } + } + // Advance month + month += 1; + if month > 12 { + month = 1; + year += 1; + } + // Safety: don't loop forever (covers 4 years = 48 months max) + if year > after.year() + 4 { + break; + } + } + None +} + +fn scan_dir(dir: &Path, scope: &str) -> Vec { + let mut defs = Vec::new(); + let Ok(entries) = std::fs::read_dir(dir) else { + return defs; + }; + for entry in entries.flatten() { + let path = entry.path(); + if path.extension().and_then(|e| e.to_str()) == Some("yaml") { + if let Ok(content) = std::fs::read_to_string(&path) { + match serde_yml::from_str::(&content) { + Ok(mut def) => { + if let Err(e) = def.validate() { + eprintln!("warning: invalid schedule {}: {e}", path.display()); + continue; + } + def.scope = scope.to_string(); + defs.push(def); + } + Err(e) => { + eprintln!("warning: failed to parse {}: {e}", path.display()); + } + } + } + } + } + defs.sort_by(|a, b| a.name.cmp(&b.name)); + defs +} + +fn find_in_dir(dir: &Path, name: &str, scope: &str) -> Option { + let path = dir.join(format!("{name}.yaml")); + if path.is_file() { + if let Ok(content) = std::fs::read_to_string(&path) { + if let Ok(mut def) = serde_yml::from_str::(&content) { + if def.validate().is_err() { + return None; + } + def.scope = scope.to_string(); + return Some(def); + } + } + } + scan_dir(dir, scope) + .into_iter() + .find(|def| def.name == name) +} + +#[cfg(test)] +mod tests; diff --git a/crates/pu-core/src/schedule_def/tests.rs b/crates/pu-core/src/schedule_def/tests.rs new file mode 100644 index 0000000..124920d --- /dev/null +++ b/crates/pu-core/src/schedule_def/tests.rs @@ -0,0 +1,442 @@ +use super::*; +use tempfile::TempDir; + +fn isolate_home(tmp: &TempDir) { + paths::set_home_override(Some(tmp.path().to_path_buf())); +} + +fn make_trigger() -> ScheduleTrigger { + ScheduleTrigger::AgentDef { + name: "security-review".to_string(), + } +} + +fn make_schedule_def(name: &str) -> ScheduleDef { + ScheduleDef { + name: name.to_string(), + enabled: true, + recurrence: Recurrence::Daily, + start_at: Utc::now(), + next_run: None, + trigger: make_trigger(), + project_root: "/projects/myapp".to_string(), + target: String::new(), + root: true, + agent_name: None, + scope: String::new(), + created_at: Utc::now(), + } +} + +// --- Deserialization (REQ-SCHED-001) --- + +#[test] +fn given_schedule_def_yaml_should_deserialize() { + let yaml = r#" +name: nightly-review +enabled: true +recurrence: daily +start_at: "2025-01-01T03:00:00Z" +trigger: + type: agent_def + name: security-review +project_root: /projects/myapp +created_at: "2025-01-01T00:00:00Z" +"#; + let def: ScheduleDef = serde_yml::from_str(yaml).unwrap(); + assert_eq!(def.name, "nightly-review"); + assert!(def.enabled); + assert_eq!(def.recurrence, Recurrence::Daily); + assert!( + matches!(def.trigger, ScheduleTrigger::AgentDef { ref name } if name == "security-review") + ); + assert_eq!(def.project_root, "/projects/myapp"); +} + +#[test] +fn given_minimal_schedule_yaml_should_use_defaults() { + let yaml = r#" +name: quick +start_at: "2025-06-01T12:00:00Z" +trigger: + type: agent_def + name: test +project_root: /tmp +created_at: "2025-06-01T00:00:00Z" +"#; + let def: ScheduleDef = serde_yml::from_str(yaml).unwrap(); + assert!(def.enabled); // default true + assert_eq!(def.recurrence, Recurrence::None); // default none + assert_eq!(def.target, ""); // default empty + assert!(def.next_run.is_none()); // default none + assert!(def.root); // default true (backward compat) + assert!(def.agent_name.is_none()); // default none +} + +#[test] +fn given_schedule_with_worktree_fields_should_round_trip() { + let yaml = r#" +name: overnight-build +start_at: "2025-06-01T22:30:00Z" +trigger: + type: inline_prompt + prompt: "build a feature" +project_root: /projects/myapp +root: false +agent_name: overnight-build +created_at: "2025-06-01T00:00:00Z" +"#; + let def: ScheduleDef = serde_yml::from_str(yaml).unwrap(); + assert!(!def.root); + assert_eq!(def.agent_name.as_deref(), Some("overnight-build")); + + // Round-trip through YAML + let serialized = serde_yml::to_string(&def).unwrap(); + let reparsed: ScheduleDef = serde_yml::from_str(&serialized).unwrap(); + assert!(!reparsed.root); + assert_eq!(reparsed.agent_name.as_deref(), Some("overnight-build")); +} + +// --- Validation --- + +#[test] +fn given_root_true_with_no_agent_name_should_validate() { + let def = make_schedule_def("test"); + assert!(def.validate().is_ok()); +} + +#[test] +fn given_root_true_with_agent_name_should_reject() { + let mut def = make_schedule_def("test"); + def.agent_name = Some("bad".to_string()); + assert!(def.validate().is_err()); +} + +#[test] +fn given_root_true_with_empty_agent_name_should_reject() { + let mut def = make_schedule_def("test"); + def.agent_name = Some(String::new()); + assert!(def.validate().is_err()); +} + +#[test] +fn given_root_false_with_agent_name_should_validate() { + let mut def = make_schedule_def("test"); + def.root = false; + def.agent_name = Some("my-worktree".to_string()); + assert!(def.validate().is_ok()); +} + +#[test] +fn given_root_false_with_no_agent_name_should_reject() { + let mut def = make_schedule_def("test"); + def.root = false; + assert!(def.validate().is_err()); +} + +#[test] +fn given_root_false_with_empty_agent_name_should_reject() { + let mut def = make_schedule_def("test"); + def.root = false; + def.agent_name = Some(String::new()); + assert!(def.validate().is_err()); +} + +#[test] +fn given_trigger_agent_def_should_round_trip() { + let trigger = ScheduleTrigger::AgentDef { + name: "reviewer".to_string(), + }; + let yaml = serde_yml::to_string(&trigger).unwrap(); + let parsed: ScheduleTrigger = serde_yml::from_str(&yaml).unwrap(); + assert_eq!(parsed, trigger); +} + +#[test] +fn given_trigger_swarm_def_with_vars_should_round_trip() { + let mut vars = HashMap::new(); + vars.insert("branch".to_string(), "main".to_string()); + let trigger = ScheduleTrigger::SwarmDef { + name: "full-stack".to_string(), + vars, + }; + let yaml = serde_yml::to_string(&trigger).unwrap(); + let parsed: ScheduleTrigger = serde_yml::from_str(&yaml).unwrap(); + assert_eq!(parsed, trigger); +} + +#[test] +fn given_trigger_inline_prompt_should_round_trip() { + let trigger = ScheduleTrigger::InlinePrompt { + prompt: "Review all deps".to_string(), + agent: "claude".to_string(), + }; + let yaml = serde_yml::to_string(&trigger).unwrap(); + let parsed: ScheduleTrigger = serde_yml::from_str(&yaml).unwrap(); + assert_eq!(parsed, trigger); +} + +// --- CRUD (REQ-SCHED-002 through REQ-SCHED-006) --- + +#[test] +fn given_local_and_global_schedule_defs_should_list_local_first() { + let tmp = TempDir::new().unwrap(); + isolate_home(&tmp); + let root = tmp.path(); + let local_dir = paths::schedules_dir(root); + std::fs::create_dir_all(&local_dir).unwrap(); + + let mut def = make_schedule_def("nightly"); + save_schedule_def(&local_dir, &def).unwrap(); + def.name = "weekly".to_string(); + save_schedule_def(&local_dir, &def).unwrap(); + + let defs = list_schedule_defs(root); + assert_eq!(defs.len(), 2); + assert_eq!(defs[0].name, "nightly"); + assert_eq!(defs[1].name, "weekly"); + assert_eq!(defs[0].scope, "local"); +} + +#[test] +fn given_schedule_def_name_should_find_by_name() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + let local_dir = paths::schedules_dir(root); + std::fs::create_dir_all(&local_dir).unwrap(); + + let def = make_schedule_def("nightly"); + save_schedule_def(&local_dir, &def).unwrap(); + + let found = find_schedule_def(root, "nightly"); + assert!(found.is_some()); + assert_eq!(found.unwrap().name, "nightly"); +} + +#[test] +fn given_no_schedule_defs_should_return_empty_list() { + let tmp = TempDir::new().unwrap(); + isolate_home(&tmp); + let defs = list_schedule_defs(tmp.path()); + assert!(defs.is_empty()); +} + +#[test] +fn given_schedule_def_should_save_and_load() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().join("schedules"); + let def = make_schedule_def("test-schedule"); + save_schedule_def(&dir, &def).unwrap(); + + let path = dir.join("test-schedule.yaml"); + assert!(path.is_file()); + + let content = std::fs::read_to_string(&path).unwrap(); + let loaded: ScheduleDef = serde_yml::from_str(&content).unwrap(); + assert_eq!(loaded.name, "test-schedule"); + assert_eq!(loaded.recurrence, Recurrence::Daily); +} + +#[test] +fn given_invalid_name_should_reject() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().join("schedules"); + let mut def = make_schedule_def("../evil"); + def.name = "../evil".to_string(); + assert!(save_schedule_def(&dir, &def).is_err()); +} + +#[test] +fn given_existing_schedule_def_should_delete_and_return_true() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().join("schedules"); + std::fs::create_dir_all(&dir).unwrap(); + let def = make_schedule_def("nightly"); + save_schedule_def(&dir, &def).unwrap(); + + let deleted = delete_schedule_def(&dir, "nightly").unwrap(); + assert!(deleted); + assert!(!dir.join("nightly.yaml").exists()); +} + +#[test] +fn given_nonexistent_schedule_def_should_return_false() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().join("schedules"); + std::fs::create_dir_all(&dir).unwrap(); + + let deleted = delete_schedule_def(&dir, "nonexistent").unwrap(); + assert!(!deleted); +} + +#[test] +fn given_duplicate_name_in_local_and_global_should_prefer_local() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path(); + let local_dir = paths::schedules_dir(root); + std::fs::create_dir_all(&local_dir).unwrap(); + + let def = make_schedule_def("nightly"); + save_schedule_def(&local_dir, &def).unwrap(); + + let found = find_schedule_def(root, "nightly").unwrap(); + assert_eq!(found.scope, "local"); +} + +#[test] +fn given_empty_project_root_should_reject() { + let tmp = TempDir::new().unwrap(); + let dir = tmp.path().join("schedules"); + let mut def = make_schedule_def("test"); + def.project_root = String::new(); + assert!(save_schedule_def(&dir, &def).is_err()); +} + +// --- Recurrence calculator (REQ-SCHED-010 through REQ-SCHED-018) --- + +fn utc(y: i32, m: u32, d: u32, h: u32, min: u32, s: u32) -> DateTime { + Utc.with_ymd_and_hms(y, m, d, h, min, s).unwrap() +} + +#[test] +fn given_none_recurrence_before_base_should_return_base() { + let base = utc(2025, 6, 15, 10, 0, 0); + let after = utc(2025, 6, 14, 10, 0, 0); + assert_eq!(next_occurrence(base, &Recurrence::None, after), Some(base)); +} + +#[test] +fn given_none_recurrence_after_base_should_return_none() { + let base = utc(2025, 6, 15, 10, 0, 0); + let after = utc(2025, 6, 16, 10, 0, 0); + assert_eq!(next_occurrence(base, &Recurrence::None, after), None); +} + +#[test] +fn given_hourly_recurrence_should_return_next_hour() { + let base = utc(2025, 6, 15, 10, 30, 0); + let after = utc(2025, 6, 15, 11, 0, 0); + let next = next_occurrence(base, &Recurrence::Hourly, after).unwrap(); + assert_eq!(next, utc(2025, 6, 15, 11, 30, 0)); +} + +#[test] +fn given_hourly_after_same_minute_should_advance_one_hour() { + let base = utc(2025, 6, 15, 10, 30, 0); + let after = utc(2025, 6, 15, 10, 30, 0); + let next = next_occurrence(base, &Recurrence::Hourly, after).unwrap(); + assert_eq!(next, utc(2025, 6, 15, 11, 30, 0)); +} + +#[test] +fn given_daily_recurrence_should_return_next_day() { + let base = utc(2025, 6, 15, 3, 0, 0); + let after = utc(2025, 6, 15, 4, 0, 0); + let next = next_occurrence(base, &Recurrence::Daily, after).unwrap(); + assert_eq!(next, utc(2025, 6, 16, 3, 0, 0)); +} + +#[test] +fn given_daily_before_time_today_should_return_today() { + let base = utc(2025, 6, 15, 15, 0, 0); + let after = utc(2025, 6, 15, 10, 0, 0); + let next = next_occurrence(base, &Recurrence::Daily, after).unwrap(); + assert_eq!(next, utc(2025, 6, 15, 15, 0, 0)); +} + +#[test] +fn given_weekdays_on_friday_should_skip_to_monday() { + // 2025-06-13 is a Friday + let base = utc(2025, 6, 13, 9, 0, 0); + let after = utc(2025, 6, 13, 10, 0, 0); + let next = next_occurrence(base, &Recurrence::Weekdays, after).unwrap(); + // Should skip to Monday 2025-06-16 + assert_eq!(next, utc(2025, 6, 16, 9, 0, 0)); + assert_eq!(next.weekday(), Weekday::Mon); +} + +#[test] +fn given_weekdays_on_saturday_should_skip_to_monday() { + // 2025-06-14 is a Saturday + let base = utc(2025, 6, 14, 9, 0, 0); + let after = utc(2025, 6, 14, 0, 0, 0); + let next = next_occurrence(base, &Recurrence::Weekdays, after).unwrap(); + assert_eq!(next, utc(2025, 6, 16, 9, 0, 0)); + assert_eq!(next.weekday(), Weekday::Mon); +} + +#[test] +fn given_weekdays_on_sunday_should_skip_to_monday() { + // 2025-06-15 is a Sunday + let base = utc(2025, 6, 15, 9, 0, 0); + let after = utc(2025, 6, 15, 0, 0, 0); + let next = next_occurrence(base, &Recurrence::Weekdays, after).unwrap(); + assert_eq!(next, utc(2025, 6, 16, 9, 0, 0)); + assert_eq!(next.weekday(), Weekday::Mon); +} + +#[test] +fn given_weekdays_on_wednesday_should_return_thursday() { + // 2025-06-11 is a Wednesday + let base = utc(2025, 6, 11, 9, 0, 0); + let after = utc(2025, 6, 11, 10, 0, 0); + let next = next_occurrence(base, &Recurrence::Weekdays, after).unwrap(); + assert_eq!(next, utc(2025, 6, 12, 9, 0, 0)); + assert_eq!(next.weekday(), Weekday::Thu); +} + +#[test] +fn given_weekly_should_return_same_weekday_next_week() { + // 2025-06-11 is a Wednesday + let base = utc(2025, 6, 11, 14, 0, 0); + let after = utc(2025, 6, 11, 15, 0, 0); + let next = next_occurrence(base, &Recurrence::Weekly, after).unwrap(); + assert_eq!(next, utc(2025, 6, 18, 14, 0, 0)); + assert_eq!(next.weekday(), Weekday::Wed); +} + +#[test] +fn given_weekly_same_day_before_time_should_return_same_day() { + // 2025-06-11 is a Wednesday + let base = utc(2025, 6, 11, 14, 0, 0); + let after = utc(2025, 6, 11, 10, 0, 0); + let next = next_occurrence(base, &Recurrence::Weekly, after).unwrap(); + assert_eq!(next, utc(2025, 6, 11, 14, 0, 0)); +} + +#[test] +fn given_monthly_should_return_same_day_next_month() { + let base = utc(2025, 6, 15, 3, 0, 0); + let after = utc(2025, 6, 15, 4, 0, 0); + let next = next_occurrence(base, &Recurrence::Monthly, after).unwrap(); + assert_eq!(next, utc(2025, 7, 15, 3, 0, 0)); +} + +#[test] +fn given_monthly_on_31st_should_skip_short_months() { + let base = utc(2025, 1, 31, 3, 0, 0); + let after = utc(2025, 1, 31, 4, 0, 0); + let next = next_occurrence(base, &Recurrence::Monthly, after).unwrap(); + // Feb has no 31st, March does + assert_eq!(next, utc(2025, 3, 31, 3, 0, 0)); +} + +#[test] +fn given_monthly_on_29th_should_skip_non_leap_feb() { + let base = utc(2025, 1, 29, 3, 0, 0); + let after = utc(2025, 1, 29, 4, 0, 0); + let next = next_occurrence(base, &Recurrence::Monthly, after).unwrap(); + // 2025 is not a leap year, Feb has no 29th + assert_eq!(next, utc(2025, 3, 29, 3, 0, 0)); +} + +#[test] +fn given_daily_with_after_before_start_at_should_not_precede_start_at() { + // start_at is in the future, after is now (before start_at) + let base = utc(2025, 6, 20, 9, 0, 0); + let after = utc(2025, 6, 15, 10, 0, 0); + let next = next_occurrence(base, &Recurrence::Daily, after).unwrap(); + // Should return start_at itself, never a date before it + assert!(next >= base); + assert_eq!(next, utc(2025, 6, 20, 9, 0, 0)); +} From 6d0ba6f33571036f717b64dd740b86d9bde9f1ed Mon Sep 17 00:00:00 2001 From: 2witstudios <2witstudios@gmail.com> Date: Tue, 10 Mar 2026 17:19:13 -0500 Subject: [PATCH 2/3] fix: Address CodeRabbit review feedback - Add name validation to find_schedule_def() to prevent path traversal - Fix next_none_occurrence() to use exclusive-after semantics (< vs <=) - Update tests to properly exercise global schedule directories - Add test for next_none_occurrence when after == base Addresses all 3 actionable comments from CodeRabbit review. Co-Authored-By: Claude Opus 4.6 --- crates/pu-core/src/schedule_def/mod.rs | 5 +- crates/pu-core/src/schedule_def/tests.rs | 62 ++++++++-- scripts/churn | 148 +++++++++++++++++++++++ 3 files changed, 201 insertions(+), 14 deletions(-) create mode 100755 scripts/churn diff --git a/crates/pu-core/src/schedule_def/mod.rs b/crates/pu-core/src/schedule_def/mod.rs index 90f7bb7..cd829a7 100644 --- a/crates/pu-core/src/schedule_def/mod.rs +++ b/crates/pu-core/src/schedule_def/mod.rs @@ -120,6 +120,9 @@ pub fn list_schedule_defs(project_root: &Path) -> Vec { /// Find a schedule definition by name. Checks local first, then global. pub fn find_schedule_def(project_root: &Path, name: &str) -> Option { + if crate::validation::validate_name(name).is_err() { + return None; + } let local_dir = paths::schedules_dir(project_root); if local_dir.is_dir() { if let Some(def) = find_in_dir(&local_dir, name, "local") { @@ -197,7 +200,7 @@ fn naive_at_base_time(base: DateTime, after: DateTime) -> chrono::Naiv } fn next_none_occurrence(base: DateTime, after: DateTime) -> Option> { - if after <= base { Some(base) } else { None } + if after < base { Some(base) } else { None } } fn next_hourly_occurrence(base: DateTime, after: DateTime) -> Option> { diff --git a/crates/pu-core/src/schedule_def/tests.rs b/crates/pu-core/src/schedule_def/tests.rs index 124920d..1d6457f 100644 --- a/crates/pu-core/src/schedule_def/tests.rs +++ b/crates/pu-core/src/schedule_def/tests.rs @@ -180,22 +180,36 @@ fn given_trigger_inline_prompt_should_round_trip() { #[test] fn given_local_and_global_schedule_defs_should_list_local_first() { - let tmp = TempDir::new().unwrap(); - isolate_home(&tmp); - let root = tmp.path(); - let local_dir = paths::schedules_dir(root); + // Use separate temp dirs: one for project root (local), one for HOME (global) + let home_tmp = TempDir::new().unwrap(); + let project_tmp = TempDir::new().unwrap(); + isolate_home(&home_tmp); + + let project_root = project_tmp.path(); + let local_dir = paths::schedules_dir(project_root); std::fs::create_dir_all(&local_dir).unwrap(); + // Write local schedules let mut def = make_schedule_def("nightly"); save_schedule_def(&local_dir, &def).unwrap(); def.name = "weekly".to_string(); save_schedule_def(&local_dir, &def).unwrap(); - let defs = list_schedule_defs(root); - assert_eq!(defs.len(), 2); + // Write a global schedule + let global_dir = paths::global_schedules_dir().unwrap(); + std::fs::create_dir_all(&global_dir).unwrap(); + def.name = "global-only".to_string(); + save_schedule_def(&global_dir, &def).unwrap(); + + let defs = list_schedule_defs(project_root); + assert_eq!(defs.len(), 3); + // Local schedules come first (sorted), then global (appended) assert_eq!(defs[0].name, "nightly"); - assert_eq!(defs[1].name, "weekly"); assert_eq!(defs[0].scope, "local"); + assert_eq!(defs[1].name, "weekly"); + assert_eq!(defs[1].scope, "local"); + assert_eq!(defs[2].name, "global-only"); + assert_eq!(defs[2].scope, "global"); } #[test] @@ -271,16 +285,30 @@ fn given_nonexistent_schedule_def_should_return_false() { #[test] fn given_duplicate_name_in_local_and_global_should_prefer_local() { - let tmp = TempDir::new().unwrap(); - let root = tmp.path(); - let local_dir = paths::schedules_dir(root); + // Use separate temp dirs: one for project root (local), one for HOME (global) + let home_tmp = TempDir::new().unwrap(); + let project_tmp = TempDir::new().unwrap(); + isolate_home(&home_tmp); + + let project_root = project_tmp.path(); + let local_dir = paths::schedules_dir(project_root); std::fs::create_dir_all(&local_dir).unwrap(); - let def = make_schedule_def("nightly"); - save_schedule_def(&local_dir, &def).unwrap(); + // Write global schedule first + let global_dir = paths::global_schedules_dir().unwrap(); + std::fs::create_dir_all(&global_dir).unwrap(); + let mut global_def = make_schedule_def("nightly"); + global_def.project_root = "/global/path".to_string(); + save_schedule_def(&global_dir, &global_def).unwrap(); + + // Write local schedule with same name + let local_def = make_schedule_def("nightly"); + save_schedule_def(&local_dir, &local_def).unwrap(); - let found = find_schedule_def(root, "nightly").unwrap(); + // Local should take priority + let found = find_schedule_def(project_root, "nightly").unwrap(); assert_eq!(found.scope, "local"); + assert_eq!(found.project_root, "/projects/myapp"); // local's project_root } #[test] @@ -312,6 +340,14 @@ fn given_none_recurrence_after_base_should_return_none() { assert_eq!(next_occurrence(base, &Recurrence::None, after), None); } +#[test] +fn given_none_recurrence_at_base_should_return_none() { + // Exclusive-after semantics: after == base means already ran, so None + let base = utc(2025, 6, 15, 10, 0, 0); + let after = base; + assert_eq!(next_occurrence(base, &Recurrence::None, after), None); +} + #[test] fn given_hourly_recurrence_should_return_next_hour() { let base = utc(2025, 6, 15, 10, 30, 0); diff --git a/scripts/churn b/scripts/churn new file mode 100755 index 0000000..2a06294 --- /dev/null +++ b/scripts/churn @@ -0,0 +1,148 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Churn hotspot analysis for Rust + Swift +# Score = LoC x churn x cyclomatic_complexity +# Requires: scc (brew install scc), jq + +# --- Defaults --- +DAYS=90 +TOP=20 +MIN_LOC=50 +NO_TESTS=false +JSON_OUT=false + +# --- Arg parsing --- +usage() { + cat <<'USAGE' +Usage: scripts/churn [OPTIONS] + +Rank files by hotspot score (LoC x churn x complexity). + +Options: + --days N Git log window in days (default: 90) + --top N Show top N results (default: 20) + --min-loc N Skip files with fewer code lines (default: 50) + --no-tests Exclude test files + --json Output as JSON array + -h, --help Show this help +USAGE + exit 0 +} + +while [[ $# -gt 0 ]]; do + case "$1" in + --days) DAYS="$2"; shift 2 ;; + --top) TOP="$2"; shift 2 ;; + --min-loc) MIN_LOC="$2"; shift 2 ;; + --no-tests) NO_TESTS=true; shift ;; + --json) JSON_OUT=true; shift ;; + -h|--help) usage ;; + *) echo "Unknown option: $1" >&2; exit 1 ;; + esac +done + +# --- Dependency check --- +missing="" +command -v scc >/dev/null 2>&1 || missing="$missing scc" +command -v jq >/dev/null 2>&1 || missing="$missing jq" +if [[ -n "$missing" ]]; then + echo "Missing dependencies:$missing" >&2 + echo "Install with: brew install$missing" >&2 + exit 1 +fi + +# --- Temp files --- +tmpdir=$(mktemp -d) +trap 'rm -rf "$tmpdir"' EXIT + +churn_file="$tmpdir/churn.tsv" +scc_file="$tmpdir/scc.tsv" +joined_file="$tmpdir/joined.tsv" + +# --- Git churn --- +# Count how many commits touched each .rs/.swift file in the window +git log --since="${DAYS} days ago" --name-only --pretty=format: --diff-filter=ACMR \ + -- '*.rs' '*.swift' \ + | grep -E '\.(rs|swift)$' \ + | sort \ + | uniq -c \ + | awk '{ print $2 "\t" $1 }' \ + > "$churn_file" + +if [[ ! -s "$churn_file" ]]; then + echo "No .rs/.swift file changes in the last ${DAYS} days." >&2 + exit 0 +fi + +# --- scc metrics --- +# Get per-file metrics as JSON, excluding build/worktree dirs +scc --by-file --format json --include-ext rs,swift \ + --exclude-dir .pu,target,DerivedData,build . \ + | jq -r '.[] | .Files[]? | [.Location, (.Code | tostring), (.Complexity | tostring)] | @tsv' \ + | sed 's|^\./||' \ + > "$scc_file" + +# --- Join churn + scc, compute score, filter --- +# For each file with churn, look up scc metrics and compute gzip density +while IFS=$'\t' read -r file churn; do + # Skip files that no longer exist (deleted/renamed) + [[ ! -f "$file" ]] && continue + + # Look up scc metrics + scc_line=$(grep -F "$file" "$scc_file" | head -1) || true + [[ -z "$scc_line" ]] && continue + + loc=$(echo "$scc_line" | cut -f2) + cx=$(echo "$scc_line" | cut -f3) + + # Skip below min-loc + [[ "$loc" -lt "$MIN_LOC" ]] && continue + + # Skip tests if requested + if $NO_TESTS; then + base=$(basename "$file") + case "$base" in + *test*|*Test*|*_test.*|*_tests.*|*Tests.*) continue ;; + esac + fi + + # Score + score=$(( loc * churn * cx )) + [[ "$score" -eq 0 ]] && continue + + # Gzip density + raw_size=$(wc -c < "$file" | tr -d ' ') + if [[ "$raw_size" -gt 0 ]]; then + gz_size=$(gzip -c "$file" | wc -c | tr -d ' ') + density=$(( gz_size * 100 / raw_size )) + else + density=0 + fi + + printf '%d\t%s\t%s\t%s\t%d%%\t%s\n' "$score" "$loc" "$churn" "$cx" "$density" "$file" +done < "$churn_file" | sort -t$'\t' -k1 -nr | head -n "$TOP" > "$joined_file" + +if [[ ! -s "$joined_file" ]]; then + echo "No hotspots found (try --days with a wider window or --min-loc with a lower threshold)." >&2 + exit 0 +fi + +# --- Output --- +if $JSON_OUT; then + jq -Rsn ' + [inputs | split("\n")[] | select(length > 0) | split("\t") | + { score: (.[0] | tonumber), + loc: (.[1] | tonumber), + churn: (.[2] | tonumber), + complexity: (.[3] | tonumber), + density: .[4], + file: .[5] }] + ' < "$joined_file" +else + printf ' %7s %5s %5s %3s %7s %s\n' "Score" "LoC" "Churn" "Cx" "Density" "File" + printf ' %7s %5s %5s %3s %7s %s\n' "───────" "─────" "─────" "───" "───────" "──────────────────────────────────────" + while IFS=$'\t' read -r score loc churn cx density file; do + printf ' %7s %5s %5s %3s %7s %s\n' "$score" "$loc" "$churn" "$cx" "$density" "$file" + done < "$joined_file" +fi From 3250169c6b3baeebb19e632e49898125c872b93d Mon Sep 17 00:00:00 2001 From: 2witstudios <2witstudios@gmail.com> Date: Tue, 10 Mar 2026 17:24:57 -0500 Subject: [PATCH 3/3] chore: Remove accidentally committed churn script The scripts/churn file was unrelated to this PR's purpose (schedule_def refactoring). Co-Authored-By: Claude Opus 4.6 --- scripts/churn | 148 -------------------------------------------------- 1 file changed, 148 deletions(-) delete mode 100755 scripts/churn diff --git a/scripts/churn b/scripts/churn deleted file mode 100755 index 2a06294..0000000 --- a/scripts/churn +++ /dev/null @@ -1,148 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -# Churn hotspot analysis for Rust + Swift -# Score = LoC x churn x cyclomatic_complexity -# Requires: scc (brew install scc), jq - -# --- Defaults --- -DAYS=90 -TOP=20 -MIN_LOC=50 -NO_TESTS=false -JSON_OUT=false - -# --- Arg parsing --- -usage() { - cat <<'USAGE' -Usage: scripts/churn [OPTIONS] - -Rank files by hotspot score (LoC x churn x complexity). - -Options: - --days N Git log window in days (default: 90) - --top N Show top N results (default: 20) - --min-loc N Skip files with fewer code lines (default: 50) - --no-tests Exclude test files - --json Output as JSON array - -h, --help Show this help -USAGE - exit 0 -} - -while [[ $# -gt 0 ]]; do - case "$1" in - --days) DAYS="$2"; shift 2 ;; - --top) TOP="$2"; shift 2 ;; - --min-loc) MIN_LOC="$2"; shift 2 ;; - --no-tests) NO_TESTS=true; shift ;; - --json) JSON_OUT=true; shift ;; - -h|--help) usage ;; - *) echo "Unknown option: $1" >&2; exit 1 ;; - esac -done - -# --- Dependency check --- -missing="" -command -v scc >/dev/null 2>&1 || missing="$missing scc" -command -v jq >/dev/null 2>&1 || missing="$missing jq" -if [[ -n "$missing" ]]; then - echo "Missing dependencies:$missing" >&2 - echo "Install with: brew install$missing" >&2 - exit 1 -fi - -# --- Temp files --- -tmpdir=$(mktemp -d) -trap 'rm -rf "$tmpdir"' EXIT - -churn_file="$tmpdir/churn.tsv" -scc_file="$tmpdir/scc.tsv" -joined_file="$tmpdir/joined.tsv" - -# --- Git churn --- -# Count how many commits touched each .rs/.swift file in the window -git log --since="${DAYS} days ago" --name-only --pretty=format: --diff-filter=ACMR \ - -- '*.rs' '*.swift' \ - | grep -E '\.(rs|swift)$' \ - | sort \ - | uniq -c \ - | awk '{ print $2 "\t" $1 }' \ - > "$churn_file" - -if [[ ! -s "$churn_file" ]]; then - echo "No .rs/.swift file changes in the last ${DAYS} days." >&2 - exit 0 -fi - -# --- scc metrics --- -# Get per-file metrics as JSON, excluding build/worktree dirs -scc --by-file --format json --include-ext rs,swift \ - --exclude-dir .pu,target,DerivedData,build . \ - | jq -r '.[] | .Files[]? | [.Location, (.Code | tostring), (.Complexity | tostring)] | @tsv' \ - | sed 's|^\./||' \ - > "$scc_file" - -# --- Join churn + scc, compute score, filter --- -# For each file with churn, look up scc metrics and compute gzip density -while IFS=$'\t' read -r file churn; do - # Skip files that no longer exist (deleted/renamed) - [[ ! -f "$file" ]] && continue - - # Look up scc metrics - scc_line=$(grep -F "$file" "$scc_file" | head -1) || true - [[ -z "$scc_line" ]] && continue - - loc=$(echo "$scc_line" | cut -f2) - cx=$(echo "$scc_line" | cut -f3) - - # Skip below min-loc - [[ "$loc" -lt "$MIN_LOC" ]] && continue - - # Skip tests if requested - if $NO_TESTS; then - base=$(basename "$file") - case "$base" in - *test*|*Test*|*_test.*|*_tests.*|*Tests.*) continue ;; - esac - fi - - # Score - score=$(( loc * churn * cx )) - [[ "$score" -eq 0 ]] && continue - - # Gzip density - raw_size=$(wc -c < "$file" | tr -d ' ') - if [[ "$raw_size" -gt 0 ]]; then - gz_size=$(gzip -c "$file" | wc -c | tr -d ' ') - density=$(( gz_size * 100 / raw_size )) - else - density=0 - fi - - printf '%d\t%s\t%s\t%s\t%d%%\t%s\n' "$score" "$loc" "$churn" "$cx" "$density" "$file" -done < "$churn_file" | sort -t$'\t' -k1 -nr | head -n "$TOP" > "$joined_file" - -if [[ ! -s "$joined_file" ]]; then - echo "No hotspots found (try --days with a wider window or --min-loc with a lower threshold)." >&2 - exit 0 -fi - -# --- Output --- -if $JSON_OUT; then - jq -Rsn ' - [inputs | split("\n")[] | select(length > 0) | split("\t") | - { score: (.[0] | tonumber), - loc: (.[1] | tonumber), - churn: (.[2] | tonumber), - complexity: (.[3] | tonumber), - density: .[4], - file: .[5] }] - ' < "$joined_file" -else - printf ' %7s %5s %5s %3s %7s %s\n' "Score" "LoC" "Churn" "Cx" "Density" "File" - printf ' %7s %5s %5s %3s %7s %s\n' "───────" "─────" "─────" "───" "───────" "──────────────────────────────────────" - while IFS=$'\t' read -r score loc churn cx density file; do - printf ' %7s %5s %5s %3s %7s %s\n' "$score" "$loc" "$churn" "$cx" "$density" "$file" - done < "$joined_file" -fi