From 1f11f3574a7beb0d748198cbde1ea222e404bc63 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Sat, 10 Oct 2026 18:26:28 -0400 Subject: [PATCH] Catch up every period within 24 hours, oldest first (F20-01) Catch-up handled only each schedule's most recent fire time, so an earlier unclaimed period still within CatchUpWindow was skipped for good. Across the Chicago spring-forward change, 01:30 on 8 March and 01:30 on 9 March 2026 are 23 hours apart; booting at 9 March 01:35 ran only 9 March. The scheduler now handles, in time order, every fire time after done that passed within CatchUpWindow, plus the most recent one even when it is older (recorded as missed, as before). Each period keeps its own claim; a failed claim stops that schedule's queue and is retried in order. Older fire times are still never looked at. Co-Authored-By: Claude Opus 5.5 --- internal/scheduler/scheduler.go | 100 ++++++++++++++++----------- internal/scheduler/scheduler_test.go | 31 +++++++++ 2 files changed, 90 insertions(+), 41 deletions(-) diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index f8a95bf..a37eaf5 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -15,13 +15,14 @@ // // # Catch-up // -// When the scheduler starts, and whenever it wakes late, it looks only at -// each schedule's most recent fire time. If that time passed within -// CatchUpWindow and the period is unclaimed, the period runs once. If it -// passed longer ago, the period is claimed without running and logged as -// missed, so it can never run later, not after a restart and not after a -// settings change moves the fire time. Fire times before the most recent -// one are never looked at. +// When the scheduler starts, and whenever it wakes late, it handles, oldest +// first, every fire time of each schedule that passed within CatchUpWindow, +// and the most recent fire time even when it passed longer ago. Each +// unclaimed period within CatchUpWindow runs once. A most recent one that +// passed longer ago is claimed without running and logged as missed, so it +// can never run later, not after a restart and not after a settings change +// moves the fire time. Other fire times older than CatchUpWindow are never +// looked at. // // # Claim failures // @@ -71,6 +72,7 @@ import ( "errors" "fmt" "log/slog" + "slices" "strings" "sync" "time" @@ -316,7 +318,7 @@ func (s *Scheduler) Register(sch Schedule) error { } // Start reads the settings and starts the scheduler, which first catches -// up on each schedule's most recent period and then fires schedules as +// up on each schedule's recent periods and then fires schedules as // their times come. Settings whose zone or rules are invalid are an error // and nothing starts. The scheduler stops when ctx is cancelled; Wait waits // for it. @@ -496,22 +498,21 @@ func (s *Scheduler) compileChange(ctx context.Context, settings household.Settin return p } -// target is one schedule's period, named by the fire time it belongs to -// and the household-local date of its period. A target whose fire time -// could not be looked up has lookup set instead, with the stretch -// (done, until] to look it up in again. +// target is one schedule's periods still to handle, oldest first, each +// named by its fire time and the household-local date of its period. A +// target whose fire times could not be looked up has lookup set instead, +// with the stretch (done, until] to look them up in again. type target struct { - it item - loc *time.Location - at time.Time - date store.LocalDate + it item + loc *time.Location + fires []fireTime lookup bool done, until time.Time } -// fire handles, for every schedule, its most recent fire time at or before -// until if it is after done; now is the current instant. It returns the +// fire handles, for every schedule, its catch-up fire times after done and +// at or before until; now is the current instant. It returns the // targets whose lookup or claim failed, to be handled again. func (s *Scheduler) fire(ctx context.Context, p *plan, done, until, now time.Time) []target { var failed []target @@ -526,21 +527,36 @@ func (s *Scheduler) fire(ctx context.Context, p *plan, done, until, now time.Tim return failed } -// fireItem handles it's most recent fire time at or before until if it is -// after done. It reports false, with the target to handle again, when the -// fire time could not be looked up or its period's claim failed. +// fireItem handles, oldest first, it's fire times after done and at or +// before until that passed within CatchUpWindow of now, and the most recent +// of them even when it passed longer ago. It reports false, with the target +// to handle again, when the fire times could not be looked up or a period's +// claim failed. func (s *Scheduler) fireItem(ctx context.Context, it item, loc *time.Location, done, until, now time.Time) (target, bool) { - f, due, err := fireBefore(ctx, it.rule, loc, until) + fs, err := fireBetween(ctx, it.rule, loc, done, until) if err != nil { s.logger.ErrorContext(ctx, "scheduler: look up fire time failed; retrying", "schedule", it.sch.label(), "error", err, "retry_in", ClaimRetry.String()) return target{it: it, loc: loc, lookup: true, done: done, until: until}, false } - if !due || !f.at.After(done) { - return target{}, true + i := 0 + for i < len(fs)-1 && now.Sub(fs[i].at) > CatchUpWindow { + i++ } - t := target{it: it, loc: loc, at: f.at, date: f.date} - return t, s.handle(ctx, t, now) + return s.handleAll(ctx, target{it: it, loc: loc, fires: fs[i:]}, now) +} + +// handleAll handles the periods of t in order, stopping at the first whose +// claim failed. It reports false, with that period and those after it, +// when one did. +func (s *Scheduler) handleAll(ctx context.Context, t target, now time.Time) (target, bool) { + for i, f := range t.fires { + if !s.handle(ctx, t.it, f, now) { + t.fires = t.fires[i:] + return t, false + } + } + return target{}, true } // retry handles targets again at now and returns those whose lookup or @@ -557,18 +573,18 @@ func (s *Scheduler) retry(ctx context.Context, targets []target, now time.Time) } continue } - if !s.handle(ctx, t, now) { - failed = append(failed, t) + if again, ok := s.handleAll(ctx, t, now); !ok { + failed = append(failed, again) } } return failed } -// handle claims the period of t and runs it, or records it as missed when -// its fire time is more than CatchUpWindow before now. It reports false +// handle claims the period of it's fire time f and runs it, or records it +// as missed when f is more than CatchUpWindow before now. It reports false // when the claim failed and the period is still unclaimed. -func (s *Scheduler) handle(ctx context.Context, t target, now time.Time) bool { - it, at, date := t.it, t.at, t.date +func (s *Scheduler) handle(ctx context.Context, it item, f fireTime, now time.Time) bool { + at, date := f.at, f.date period := Period{Key: it.rule.PeriodKey(date), FireAt: at, Date: date} if it.rule.NextWeek { period.Week = household.WeekStart(addDays(date, 7)) @@ -634,12 +650,12 @@ func (p *plan) next(ctx context.Context, now time.Time) (time.Time, bool, error) return next, !next.IsZero(), nil } -// window is how many days either side of a date fireBefore and fireAfter +// window is how many days either side of a date fireBetween and fireAfter // search: more than a week, so a weekly rule always has a fire time on // each side. const window = 8 -// atWindow is how many days either side of a date fireBefore and fireAfter +// atWindow is how many days either side of a date fireBetween and fireAfter // look up a DailyAt rule's fire times. A period firing further off is // looked up on a later pass, which comes within maxWait. const atWindow = 2 @@ -650,19 +666,21 @@ type fireTime struct { date store.LocalDate } -// fireBefore returns the latest fire time of r in zone loc at or before t. -func fireBefore(ctx context.Context, r Rule, loc *time.Location, t time.Time) (fireTime, bool, error) { +// fireBetween returns the fire times of r in zone loc after from and at or +// before t, oldest first. +func fireBetween(ctx context.Context, r Rule, loc *time.Location, from, t time.Time) ([]fireTime, error) { fs, err := fireTimes(ctx, r, loc, t) if err != nil { - return fireTime{}, false, err + return nil, err } - var best fireTime + var out []fireTime for _, f := range fs { - if !f.at.After(t) && (best.at.IsZero() || f.at.After(best.at)) { - best = f + if f.at.After(from) && !f.at.After(t) { + out = append(out, f) } } - return best, !best.at.IsZero(), nil + slices.SortFunc(out, func(a, b fireTime) int { return a.at.Compare(b.at) }) + return out, nil } // fireAfter returns the earliest fire time of r in zone loc after t. diff --git a/internal/scheduler/scheduler_test.go b/internal/scheduler/scheduler_test.go index 534eaca..4edf8b3 100644 --- a/internal/scheduler/scheduler_test.go +++ b/internal/scheduler/scheduler_test.go @@ -403,6 +403,37 @@ func TestCatchUpOnlyMostRecentPeriod(t *testing.T) { wantKeys(t, "schedule_runs", e.claimed(scheduler.DayOfReminder), []string{"2026-10-09"}) } +func TestCatchUpEveryPeriodWithin24HoursAcrossSpringForward(t *testing.T) { + // Clocks in Chicago jump 02:00 CST → 03:00 CDT on Sunday 2026-03-08, + // so 01:30 on Mar 8 and 01:30 on Mar 9 are only 23 hours apart. + e := newEnv(t) + e.update(household.SettingsPatch{ReviewTime: ptr("01:30")}) + var rec recorder + review := scheduler.Schedule{ + Name: scheduler.RotationCheck, + Rule: func(s household.Settings) scheduler.Rule { return scheduler.Daily(s.ReviewTime) }, + Run: rec.run, + } + // Boot at Mar 9 01:35: Mar 9 01:30 passed 5 minutes ago and Mar 8 + // 01:30 23h05m ago, so both run, oldest first; Mar 7 01:30, 47h05m + // ago, is not looked at. + r := e.start(at(2026, 3, 9, 1, 35), review) + wantKeys(t, "catch-up", rec.keys(), []string{"2026-03-08", "2026-03-09"}) + wantFired(t, rec.periods(), + time.Date(2026, 3, 8, 7, 30, 0, 0, time.UTC), // 01:30 CST + time.Date(2026, 3, 9, 6, 30, 0, 0, time.UTC), // 01:30 CDT + ) + wantKeys(t, "schedule_runs", e.claimed(scheduler.RotationCheck), []string{"2026-03-08", "2026-03-09"}) + r.stop() + + // A restart within the window finds both periods claimed. + var again recorder + review.Run = again.run + r2 := e.start(at(2026, 3, 9, 1, 40), review) + r2.runUntil(at(2026, 3, 9, 23, 0)) + wantKeys(t, "after restart", again.keys(), nil) +} + func TestSpringForward(t *testing.T) { // Clocks in Chicago jump 02:00 CST → 03:00 CDT on Sunday 2026-03-08. e := newEnv(t) -- 2.55.0