Fix CronScheduler: replace hardcoded interval switch with proper cron parsing

[Copilot]

module/scheduler.go only recognized 3 hardcoded cron patterns (* * * * *, 0 * * * *, 0 0 * * *), silently defaulting all other expressions to 1-minute intervals. The scheduler/ package already contained a complete cron parser (ValidateCron, defaultNextRun) that was never wired in.

Changes

• scheduler/scheduler.go: Export NextRun() as a thin wrapper around defaultNextRun() to make the cron calculation available to other packages.

• module/scheduler.go:

  • Remove ticker *time.Ticker field and the hardcoded switch block.
  • Validate expressions on Start() via scheduler.ValidateCron(); return a descriptive error on invalid input.
  • Replace fixed-interval time.Ticker with a per-iteration time.NewTimer driven by scheduler.NextRun(), so each job fires at the correct cron-specified time.

  // Before
  interval := time.Minute
  switch s.cronExpression {
  case "* * * * *": interval = time.Minute
  case "0 * * * *": interval = time.Hour
  case "0 0 * * *": interval = 24 * time.Hour
  }
  s.ticker = time.NewTicker(interval)
  
  // After
  if err := scheduler.ValidateCron(s.cronExpression); err != nil {
      return fmt.Errorf("invalid cron expression %q: %w", s.cronExpression, err)
  }
  // inside goroutine loop:
  next, _ := scheduler.NextRun(s.cronExpression, time.Now())
  timer := time.NewTimer(time.Until(next))

• module/scheduler_test.go:

  • Drop the "custom" default case and the ticker == nil assertion.
  • Add coverage for */5 * * * *, 0 9 * * 1-5 (valid expressions), invalid expression rejection, and next-run time correctness for step expressions.

Original prompt

Problem

The module/scheduler.go CronScheduler has a simplified cron expression parser that only recognizes 3 hardcoded patterns:

switch s.cronExpression {
case "* * * * *": // every minute
    interval = time.Minute
case "0 * * * *": // every hour
    interval = time.Hour
case "0 0 * * *": // every day
    interval = 24 * time.Hour
}

Any other cron expression (e.g., */5 * * * *, 0 9 * * 1-5, 30 2 1 * *) defaults to a 1-minute interval regardless of the expression, which is incorrect behavior.

Meanwhile, the scheduler/scheduler.go package has a much more complete cron implementation with ValidateCron(), defaultNextRun(), and proper field parsing including step values (*/N), ranges (1-5), and comma-separated values.

Current State

// module/scheduler.go - SIMPLIFIED (used by config-driven workflows)
interval := time.Minute
switch s.cronExpression {
case "* * * * *":
    interval = time.Minute
case "0 * * * *":
    interval = time.Hour
case "0 0 * * *":
    interval = 24 * time.Hour
}
s.ticker = time.NewTicker(interval)

// scheduler/scheduler.go - MORE COMPLETE (separate package)
func defaultNextRun(cronExpr string, from time.Time) (time.Time, error) { ... }
func ValidateCron(expr string) error { ... }

Requirements

1. Replace the hardcoded switch statement in module/scheduler.go's Start() method with proper cron scheduling logic.

2. Reuse the scheduler.ValidateCron() and scheduler.defaultNextRun() functions from the scheduler/ package, OR copy and adapt the logic to avoid a circular dependency. The preferred approach is to extract the cron parsing into a shared utility (e.g., cron/ package or within module/) that both module/scheduler.go and scheduler/scheduler.go can use.

3. Replace the fixed-interval time.Ticker with a dynamic scheduling approach:

   • After each job execution, calculate the next run time using the cron expression.
   • Use time.After(duration) or time.NewTimer(duration) to wait until the next run.
   • This ensures jobs run at the correct cron-specified times, not at fixed intervals.

4. Support standard 5-field cron expressions:

   • * * * * * (every minute)
   • */5 * * * * (every 5 minutes)
   • 0 * * * * (every hour at minute 0)
   • 30 2 * * * (daily at 2:30 AM)
   • 0 9 * * 1-5 (weekdays at 9 AM)
   • 0 0 1 * * (first of each month at midnight)
   • Ranges (1-5), comma-separated (1,3,5), and step values (*/10)

5. Add validation in NewCronScheduler() or Start() to reject invalid cron expressions with a clear error.

6. Update tests in module/scheduler_test.go:

   • Test that */5 * * * * schedules every 5 minutes (not every 1 minute).
   • Test that 0 9 * * 1-5 calculates the correct next run time.
   • Test validation rejects invalid expressions.
   • Update TestCronScheduler_CronExpressions to verify correct intervals.

7. Ensure all existing tests pass (go test ./...).

This pull request was created from Copilot chat.

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.