diff --git a/internal/store/reminders.go b/internal/store/reminders.go index fefa7bb..f7b67df 100644 --- a/internal/store/reminders.go +++ b/internal/store/reminders.go @@ -205,8 +205,30 @@ func (s *Store) ListReminders(ctx context.Context, n int) ([]Reminder, error) { // RescheduleReminder computes the next fire time for a recurring reminder and // updates next_fire_ts. Returns ErrReminderState if the reminder is not -// recurring or not pending. If no more valid fire times exist, marks it fired. +// recurring or not pending. If the schedule yields no further fire time at all, +// marks it fired. +// +// Two things the first version got wrong, both fixed 06-08-2026 (V-616). +// +// The cron expression is a WALL CLOCK statement — "0 9 * * *" is nine in the +// morning where the owner stands — but scanReminder hands back instants in UTC, +// and robfig's Next walks the calendar in the location of the time it is given. +// Computing from a UTC instant therefore produced the next 09:00 UTC, so the +// second occurrence of a daily reminder landed one UTC offset late and stayed +// there: 12:00 for a Moscow owner. Everything is converted to loc first, which +// also makes the walk DST-correct — the schedule keeps its wall-clock hour +// across a changeover instead of drifting an hour with the offset. +// +// And a missed occurrence used to KILL the reminder: any next fire earlier than +// now marked it fired, so a daemon down overnight ended a daily standup forever. +// Occurrences in the past are skipped instead, so the reminder rolls forward to +// the first one strictly after now. Skipping and not replaying is deliberate: +// the same no-backlog rule routine.DueAccepted follows. func (s *Store) RescheduleReminder(ctx context.Context, id int64, now time.Time) error { + return s.rescheduleReminderIn(ctx, id, now, time.Local) +} + +func (s *Store) rescheduleReminderIn(ctx context.Context, id int64, now time.Time, loc *time.Location) error { row := s.db.QueryRowContext(ctx, ` SELECT id, created_ts, fire_ts, next_fire_ts, payload, status, cron FROM reminders WHERE id = ?`, id) @@ -225,8 +247,14 @@ func (s *Store) RescheduleReminder(ctx context.Context, id int64, now time.Time) if err != nil { return fmt.Errorf("parse cron %q: %w", r.Cron, err) } - next := sched.Next(r.NextFireTs.Add(time.Minute)) - if next.IsZero() || next.Before(now) { + // Next is strictly after the time it is given, so the last fire cannot be + // returned again and no fudge minute is needed. The bound stops a schedule + // that somehow yields a non-advancing time from spinning here. + next := sched.Next(r.NextFireTs.In(loc)) + for i := 0; i < 4096 && !next.IsZero() && !next.After(now); i++ { + next = sched.Next(next) + } + if next.IsZero() || !next.After(now) { _, err = s.db.ExecContext(ctx, "UPDATE reminders SET status = 'fired' WHERE id = ?", id) return err } diff --git a/internal/store/reminders_reschedule_test.go b/internal/store/reminders_reschedule_test.go new file mode 100644 index 0000000..89a2af7 --- /dev/null +++ b/internal/store/reminders_reschedule_test.go @@ -0,0 +1,104 @@ +package store + +import ( + "context" + "testing" + "time" +) + +// A recurring reminder keeps its wall-clock hour in the owner's zone. The cron +// walk used to run on the UTC instant scanReminder returns, so "0 9 * * *" +// created for 09:00 Moscow rescheduled to 09:00 UTC — noon, and noon every day +// after that (V-616). +func TestRescheduleKeepsWallClockHour(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + msk := time.FixedZone("MSK", 3*60*60) + + fire := time.Date(2026, 7, 1, 9, 0, 0, 0, msk) + id, err := s.CreateReminder(ctx, fire, `{"text":"стендап"}`, "0 9 * * *") + if err != nil { + t.Fatal(err) + } + if err := s.rescheduleReminderIn(ctx, id, fire, msk); err != nil { + t.Fatalf("reschedule: %v", err) + } + + want := time.Date(2026, 7, 2, 9, 0, 0, 0, msk) + got := nextFire(t, s, id) + if !got.Equal(want) { + t.Fatalf("next fire: want %s, got %s", want, got.In(msk)) + } +} + +// A daily reminder survives a daemon that was down for days. The past +// occurrences are skipped, not replayed, and the reminder stays pending. +func TestRescheduleRollsForwardAfterDowntime(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + msk := time.FixedZone("MSK", 3*60*60) + + fire := time.Date(2026, 7, 1, 9, 0, 0, 0, msk) + id, err := s.CreateReminder(ctx, fire, `{"text":"стендап"}`, "0 9 * * *") + if err != nil { + t.Fatal(err) + } + // The box comes back three days later, mid-afternoon. + now := time.Date(2026, 7, 4, 15, 0, 0, 0, msk) + if err := s.rescheduleReminderIn(ctx, id, now, msk); err != nil { + t.Fatalf("reschedule: %v", err) + } + + rs, err := s.ListReminders(ctx, 10) + if err != nil || len(rs) != 1 { + t.Fatalf("list: %d reminders, %v", len(rs), err) + } + if rs[0].Status != ReminderPending { + t.Fatalf("status: want %s, got %s — downtime killed the recurrence", ReminderPending, rs[0].Status) + } + want := time.Date(2026, 7, 5, 9, 0, 0, 0, msk) + if got := nextFire(t, s, id); !got.Equal(want) { + t.Fatalf("next fire: want %s, got %s", want, got.In(msk)) + } + // And exactly one fire is owed, not a backlog of four. + due, err := s.DueReminders(ctx, want.Add(time.Second)) + if err != nil || len(due) != 1 { + t.Fatalf("due after roll-forward: want 1, got %d (%v)", len(due), err) + } +} + +// A daily 03:00 reminder does not drift across the spring-forward changeover: +// the hour is wall clock, so the interval is 23 hours that day, not 24. +func TestRescheduleAcrossDSTKeepsHour(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + berlin, err := time.LoadLocation("Europe/Berlin") + if err != nil { + t.Skipf("tzdata unavailable: %v", err) + } + + fire := time.Date(2027, 3, 27, 3, 0, 0, 0, berlin) // CET, day before the change + id, err := s.CreateReminder(ctx, fire, `{"text":"бэкап"}`, "0 3 * * *") + if err != nil { + t.Fatal(err) + } + if err := s.rescheduleReminderIn(ctx, id, fire, berlin); err != nil { + t.Fatalf("reschedule: %v", err) + } + got := nextFire(t, s, id).In(berlin) + if got.Hour() != 3 || got.Day() != 28 { + t.Fatalf("next fire: want 2027-03-28 03:00 local, got %s", got) + } + if d := got.Sub(fire); d != 23*time.Hour { + t.Fatalf("gap across spring forward: want 23h, got %s", d) + } +} + +func nextFire(t *testing.T, s *Store, id int64) time.Time { + t.Helper() + var ms int64 + if err := s.db.QueryRow("SELECT next_fire_ts FROM reminders WHERE id = ?", id).Scan(&ms); err != nil { + t.Fatalf("read next_fire_ts: %v", err) + } + return time.UnixMilli(ms).UTC() +}