diff --git a/apps/runloop-engine/internal/scheduler/manager.go b/apps/runloop-engine/internal/scheduler/manager.go index 7a17485..ad50cce 100644 --- a/apps/runloop-engine/internal/scheduler/manager.go +++ b/apps/runloop-engine/internal/scheduler/manager.go @@ -8,14 +8,55 @@ import ( "time" "github.com/go-co-op/gocron/v2" + "github.com/robfig/cron/v3" + "github.com/rs/zerolog/log" "github.com/runloop/runloop-engine/internal/db" "github.com/runloop/runloop-engine/internal/idgen" "github.com/runloop/runloop-engine/internal/models" "github.com/runloop/runloop-engine/internal/websocket" "github.com/runloop/runloop-engine/internal/worker" - "github.com/rs/zerolog/log" ) +// computeNextRun returns the next firing time for `schedule` in the given +// IANA `timezone`. Falls back to UTC if the timezone isn't recognised. +// +// gocron's job.NextRun() can return a zero time.Time before the job has +// fired at least once (e.g. immediately after registration). When that +// happens, callers should use this helper so the persisted next_run_at is +// always a real future instant. +func computeNextRun(schedule, timezone string) (time.Time, error) { + if schedule == "" { + return time.Time{}, fmt.Errorf("empty schedule") + } + loc, err := time.LoadLocation(timezone) + if err != nil || loc == nil { + loc = time.UTC + } + parser := cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow) + expr, err := parser.Parse(schedule) + if err != nil { + return time.Time{}, fmt.Errorf("parse cron %q: %w", schedule, err) + } + return expr.Next(time.Now().In(loc)), nil +} + +// resolveNextRun prefers gocron's NextRun() value, but falls back to a +// hand-computed cron next-fire if gocron returns zero. The fallback ensures +// callers don't persist 0001-01-01 to the database after AddJob/firing. +func (m *Manager) resolveNextRun(s *models.Scheduler, job gocron.Job) time.Time { + if job != nil { + if nr, err := job.NextRun(); err == nil && !nr.IsZero() { + return nr + } + } + if s != nil && s.Schedule != nil && *s.Schedule != "" { + if nr, err := computeNextRun(*s.Schedule, s.Timezone); err == nil { + return nr + } + } + return time.Time{} +} + // Manager manages scheduled jobs type Manager struct { db *db.Postgres @@ -144,13 +185,17 @@ func (m *Manager) AddJob(s *models.Scheduler) error { m.jobs[s.ID] = job - // Update next run time - nextRun, _ := job.NextRun() - if err := m.updateNextRun(s.ID, &nextRun); err != nil { - log.Error(). - Err(err). - Str("scheduler_id", s.ID). - Msg("Failed to update next run time") + // Update next run time. gocron's job.NextRun() can return zero before the + // first firing — fall back to hand-computed cron in that case so the UI + // never has to render "Overdue" against 0001-01-01. + nextRun := m.resolveNextRun(s, job) + if !nextRun.IsZero() { + if err := m.updateNextRun(s.ID, &nextRun); err != nil { + log.Error(). + Err(err). + Str("scheduler_id", s.ID). + Msg("Failed to update next run time") + } } log.Info(). @@ -552,8 +597,10 @@ func (m *Manager) createJobFunc(s *models.Scheduler) func() { // Update next run time m.mu.RLock() if job, exists := m.jobs[s.ID]; exists { - nextRun, _ := job.NextRun() - m.updateNextRun(s.ID, &nextRun) + nextRun := m.resolveNextRun(s, job) + if !nextRun.IsZero() { + m.updateNextRun(s.ID, &nextRun) + } } m.mu.RUnlock() } @@ -886,7 +933,6 @@ func (m *Manager) executeSchedulerDirect(ctx context.Context, s *models.Schedule return nil } - // hasFlowGraph reports whether the JSONMap looks like a flow editor // flowConfig (object with `nodes` array containing at least one entry). // SIMPLE flows that wrap a single saved-Task reference end up here too, diff --git a/apps/runloop-engine/internal/scheduler/manager_test.go b/apps/runloop-engine/internal/scheduler/manager_test.go index 6a8bb76..816f131 100644 --- a/apps/runloop-engine/internal/scheduler/manager_test.go +++ b/apps/runloop-engine/internal/scheduler/manager_test.go @@ -2,6 +2,7 @@ package scheduler import ( "testing" + "time" "github.com/runloop/runloop-engine/internal/models" ) @@ -93,3 +94,54 @@ func TestHasFlowGraph_MalformedNodeIgnored(t *testing.T) { t.Errorf("malformed node should be skipped; the HTTP node still qualifies") } } + +// computeNextRun is the fallback used when gocron's NextRun() returns zero +// after a fresh AddJob (the bug that caused every freshly-seeded scheduler +// to render as "Overdue" in the UI). The fallback parses the cron itself +// and returns the next firing in the scheduler's configured timezone. + +func TestComputeNextRun_HourlyAdvances(t *testing.T) { + got, err := computeNextRun("0 * * * *", "Asia/Bangkok") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + now := time.Now() + if !got.After(now) { + t.Errorf("next run %v should be after now %v", got, now) + } + if got.Sub(now) > time.Hour+time.Minute { + t.Errorf("hourly cron next-run should be ≤ 1h away, got %v", got.Sub(now)) + } +} + +func TestComputeNextRun_TimezoneApplies(t *testing.T) { + // Daily at 02:00 ICT — the next firing should be at hour=2 in Bangkok, + // regardless of where the test runs. + got, err := computeNextRun("0 2 * * *", "Asia/Bangkok") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + bkk, _ := time.LoadLocation("Asia/Bangkok") + if got.In(bkk).Hour() != 2 { + t.Errorf("expected hour=2 in Asia/Bangkok, got %v (= %d in BKK)", got, got.In(bkk).Hour()) + } +} + +func TestComputeNextRun_FallsBackToUTC(t *testing.T) { + got, err := computeNextRun("0 * * * *", "Not/A_Real_Zone") + if err != nil { + t.Fatalf("unexpected error on bad timezone: %v", err) + } + if got.IsZero() { + t.Error("expected a real next-run time even with a bogus timezone") + } +} + +func TestComputeNextRun_RejectsBadCron(t *testing.T) { + if _, err := computeNextRun("not a cron expression", "UTC"); err == nil { + t.Error("expected parse error for bad cron") + } + if _, err := computeNextRun("", "UTC"); err == nil { + t.Error("expected error for empty schedule") + } +}