Files
multica/server/internal/service/cron_test.go
Bohan Jiang a252f47337 fix(scheduler): advance autopilot next_run_at after each scheduled dispatch (MUL-3749) (#4618)
* fix(scheduler): advance autopilot next_run_at after each scheduled dispatch

The display-only autopilot_trigger.next_run_at column was written only on
trigger create/update and never advanced afterward, so for a recurring
schedule it froze at a past slot and the list rendered it as a 'next run'
in the past (e.g. '53m ago'). The intended AdvanceTriggerNextRun query was
dead code with zero callers.

Wire it up at the scheduler's existing post-dispatch seam (replacing the
last_fired_at-only TouchAutopilotTriggerFiredAt bump, which AdvanceTrigger-
NextRun already supersets). The advanced value is computed on the app local
clock via ComputeNextRun — the same path create/update use — so the whole
next_run_at display column is owned by one clock and stays consistent;
scheduling itself is untouched and still runs off DB time via
NextOccurrencesUTC. On a cron/timezone parse failure we fall back to the
last_fired_at-only bump.

Adds a deterministic regression test for the reported scenario (hourly
cron in America/New_York) and documents the local-clock ownership on
ComputeNextRun.

MUL-3749

Co-authored-by: multica-agent <github@multica.ai>

* fix(scheduler): floor next_run_at advance at plan_time to survive clock skew

Addresses review feedback on the next_run_at write-back (MUL-3749):

- The post-dispatch advance computed the value from time.Now() alone. The
  handler is entered only after DB time judged the plan due, so if this app
  instance's clock lags the DB clock at a period boundary, time.Now() could
  recompute the slot that just fired and next_run_at would not advance —
  the original staleness bug, at the boundary. Extract advancedNextRun,
  which anchors at max(now, plan_time) via NextOccurrenceAfterUTC so the
  written value is always strictly after the fired plan_time while still
  tracking the local clock in the normal case.
- Add scheduler-layer tests asserting the written value is strictly after
  plan_time across skew / on-slot / normal cases. The previous service-layer
  test only exercised the helper with an explicit after, not this path.
- Sync the stale ListSchedulableAutopilotTriggers comment: the scheduler
  now writes last_fired_at via AdvanceTriggerNextRun (sqlc regenerated).

MUL-3749

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
2026-06-26 18:54:51 +08:00

163 lines
5.7 KiB
Go

package service
import (
"testing"
"time"
)
// TestNextOccurrencesUTCEnumeratesInTimezone verifies the cron evaluator
// is timezone-aware on input but always returns UTC on output, and that
// the half-open (after, until] window is respected.
func TestNextOccurrencesUTCEnumeratesInTimezone(t *testing.T) {
// Every Mon-Fri at 09:00 in Asia/Shanghai (UTC+8). 09:00 CST is
// 01:00 UTC the same day.
cron := "0 9 * * MON-FRI"
tz := "Asia/Shanghai"
// 2026-06-23 is a Tuesday. Pick a Monday-Friday span:
// Monday 2026-06-22 09:00 CST = 2026-06-22T01:00:00Z
// Tuesday 2026-06-23 09:00 CST = 2026-06-23T01:00:00Z
// Wednesday 2026-06-24 09:00 CST = 2026-06-24T01:00:00Z
after := time.Date(2026, 6, 21, 0, 0, 0, 0, time.UTC) // Sunday — before Mon's first fire
until := time.Date(2026, 6, 24, 1, 0, 0, 0, time.UTC) // exactly Wed 09:00 CST
got, err := NextOccurrencesUTC(cron, tz, after, until)
if err != nil {
t.Fatalf("NextOccurrencesUTC: %v", err)
}
want := []time.Time{
time.Date(2026, 6, 22, 1, 0, 0, 0, time.UTC),
time.Date(2026, 6, 23, 1, 0, 0, 0, time.UTC),
time.Date(2026, 6, 24, 1, 0, 0, 0, time.UTC),
}
if len(got) != len(want) {
t.Fatalf("expected %d occurrences, got %d: %v", len(want), len(got), got)
}
for i, g := range got {
if !g.Equal(want[i]) {
t.Fatalf("occurrence[%d]: got %s, want %s",
i, g.Format(time.RFC3339), want[i].Format(time.RFC3339))
}
if g.Location() != time.UTC {
t.Fatalf("occurrence[%d] must be UTC, got %s", i, g.Location())
}
}
}
// TestNextOccurrencesUTCEmptyWindow asserts that an `after` >= `until`
// returns nothing without erroring — the planner relies on this for
// the "nothing due yet" branch.
func TestNextOccurrencesUTCEmptyWindow(t *testing.T) {
cron := "*/5 * * * *"
tz := "UTC"
// after AFTER until: no occurrences should be returned.
after := time.Date(2026, 6, 23, 12, 0, 0, 0, time.UTC)
until := after.Add(-time.Minute)
got, err := NextOccurrencesUTC(cron, tz, after, until)
if err != nil {
t.Fatalf("NextOccurrencesUTC: %v", err)
}
if len(got) != 0 {
t.Fatalf("expected no occurrences for empty window, got %v", got)
}
}
// TestNextOccurrencesUTCExclusiveAfter verifies that a plan_time equal
// to `after` is NOT re-emitted. This is essential for the planner: when
// the most recent stored plan_time is X, the next tick must NOT
// produce X again.
func TestNextOccurrencesUTCExclusiveAfter(t *testing.T) {
cron := "*/5 * * * *"
tz := "UTC"
// after sits exactly on a cron tick. The next emitted plan must
// be the SUBSEQUENT tick, not the same one.
after := time.Date(2026, 6, 23, 12, 0, 0, 0, time.UTC)
until := time.Date(2026, 6, 23, 12, 10, 0, 0, time.UTC)
got, err := NextOccurrencesUTC(cron, tz, after, until)
if err != nil {
t.Fatalf("NextOccurrencesUTC: %v", err)
}
want := []time.Time{
time.Date(2026, 6, 23, 12, 5, 0, 0, time.UTC),
time.Date(2026, 6, 23, 12, 10, 0, 0, time.UTC),
}
if len(got) != len(want) {
t.Fatalf("expected %d occurrences, got %d: %v", len(want), len(got), got)
}
for i, g := range got {
if !g.Equal(want[i]) {
t.Fatalf("occurrence[%d]: got %s want %s",
i, g.Format(time.RFC3339), want[i].Format(time.RFC3339))
}
}
}
// TestNextOccurrencesUTCInvalidInputs surfaces parse errors loudly so
// a bad cron expression cannot silently disable a trigger.
func TestNextOccurrencesUTCInvalidInputs(t *testing.T) {
t.Run("bad cron", func(t *testing.T) {
_, err := NextOccurrencesUTC("not a cron", "UTC", time.Now(), time.Now().Add(time.Hour))
if err == nil {
t.Fatal("expected error for bad cron expression")
}
})
t.Run("bad timezone", func(t *testing.T) {
_, err := NextOccurrencesUTC("* * * * *", "Mars/Olympus", time.Now(), time.Now().Add(time.Hour))
if err == nil {
t.Fatal("expected error for invalid timezone")
}
})
}
// TestNextOccurrenceAfterUTCIgnoresWallClock verifies that the helper
// uses the explicit `after` argument, not time.Now(). This is the key
// property the scheduler relies on: callers pass dbNow() and get
// answers consistent across app instances with skewed clocks.
func TestNextOccurrenceAfterUTCIgnoresWallClock(t *testing.T) {
cron := "30 14 * * *" // 14:30 daily in UTC
// Pretend "DB now" is at midnight UTC. The next fire is 14:30 UTC
// the same day — independent of whatever the host clock says.
after := time.Date(2026, 6, 23, 0, 0, 0, 0, time.UTC)
want := time.Date(2026, 6, 23, 14, 30, 0, 0, time.UTC)
got, err := NextOccurrenceAfterUTC(cron, "UTC", after)
if err != nil {
t.Fatalf("NextOccurrenceAfterUTC: %v", err)
}
if !got.Equal(want) {
t.Fatalf("got %s, want %s", got.Format(time.RFC3339), want.Format(time.RFC3339))
}
}
// TestNextOccurrenceAdvancesPastFiredSlot locks in the property the
// scheduler's next_run_at write-back relies on (MUL-3749): once a
// recurring trigger fires at a slot, the next computed occurrence is the
// FOLLOWING slot, strictly after the one that just fired — never the same
// instant. Regression guard for the bug where next_run_at froze at a past
// slot and the list rendered it as "53m ago". Uses the exact reported
// scenario: hourly cron in America/New_York, slot 03:00 EDT (07:00 UTC).
func TestNextOccurrenceAdvancesPastFiredSlot(t *testing.T) {
const cron = "0 * * * *" // every hour, on the hour
const tz = "America/New_York"
fired := time.Date(2026, 6, 26, 7, 0, 0, 0, time.UTC) // 03:00 EDT
want := time.Date(2026, 6, 26, 8, 0, 0, 0, time.UTC) // 04:00 EDT
got, err := NextOccurrenceAfterUTC(cron, tz, fired)
if err != nil {
t.Fatalf("NextOccurrenceAfterUTC: %v", err)
}
if !got.Equal(want) {
t.Fatalf("got %s, want %s", got.Format(time.RFC3339), want.Format(time.RFC3339))
}
if !got.After(fired) {
t.Fatalf("next occurrence %s must be strictly after the fired slot %s", got, fired)
}
}