fix(scheduler): never persist 0001-01-01 as next_run_at (Closes #1)

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,

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")
+	}
+}