fix(office): correct cron DOM/DOW OR semantics, DST fire policy, and unsatisfiable expressions (#3514)

Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com>
Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
Henry
2026-09-13 03:41:22 +08:00
committed by GitHub
parent fc64661ce0
commit e27f888876
16 changed files with 1290 additions and 258 deletions
@@ -467,7 +467,7 @@ func (r *Repository) createRoutineTables() error {
routine_id TEXT NOT NULL,
kind TEXT NOT NULL,
cron_expression TEXT DEFAULT '',
timezone TEXT DEFAULT '',
timezone TEXT DEFAULT 'UTC',
public_id TEXT DEFAULT '',
signing_mode TEXT DEFAULT '',
secret TEXT DEFAULT '',
@@ -65,6 +65,21 @@ func (r *Repository) runMigrations() error {
if err := r.migrate.Err(); err != nil {
return err
}
if err := r.backfillRoutineTriggerTimezones(); err != nil {
return err
}
return nil
}
// backfillRoutineTriggerTimezones sets an explicit "UTC" on cron triggers
// created before the column default changed from empty to 'UTC', so the
// empty string no longer means anything at read time.
func (r *Repository) backfillRoutineTriggerTimezones() error {
if _, err := r.db.Exec(
`UPDATE office_routine_triggers SET timezone = 'UTC' WHERE kind = 'cron' AND timezone = ''`,
); err != nil {
return fmt.Errorf("office_routine_triggers.timezone backfill: %w", err)
}
return nil
}
@@ -0,0 +1,68 @@
package sqlite_test
import (
"context"
"testing"
"github.com/kandev/kandev/internal/office/models"
"github.com/kandev/kandev/internal/office/repository/sqlite"
taskrepo "github.com/kandev/kandev/internal/task/repository/sqlite"
"github.com/kandev/kandev/internal/testutil"
)
// TestPostgresBackfillRoutineTriggerTimezones_LegacyEmptyTimezoneBecomesUTC
// is the PostgreSQL replay twin of the SQLite backfill test. It skips unless
// KANDEV_TEST_POSTGRES_DSN is set.
func TestPostgresBackfillRoutineTriggerTimezones_LegacyEmptyTimezoneBecomesUTC(t *testing.T) {
db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t))
ctx := context.Background()
if _, err := taskrepo.NewWithDB(db, db, nil); err != nil {
t.Fatalf("init task repo: %v", err)
}
repo, err := sqlite.NewWithDB(db, db, nil)
if err != nil {
t.Fatalf("first init: %v", err)
}
routine := &models.Routine{
WorkspaceID: "ws-postgres",
Name: "Legacy Cron PostgreSQL",
TaskTemplate: "{}",
Status: "active",
ConcurrencyPolicy: "skip_if_active",
Variables: "{}",
}
if err := repo.CreateRoutine(ctx, routine); err != nil {
t.Fatalf("create routine: %v", err)
}
trigger := &models.RoutineTrigger{
RoutineID: routine.ID,
Kind: "cron",
CronExpression: "0 9 * * *",
Timezone: "UTC",
Enabled: true,
}
if err := repo.CreateRoutineTrigger(ctx, trigger); err != nil {
t.Fatalf("create trigger: %v", err)
}
if _, err := db.ExecContext(ctx,
`UPDATE office_routine_triggers SET timezone = '' WHERE id = $1`, trigger.ID,
); err != nil {
t.Fatalf("simulate legacy empty timezone: %v", err)
}
if _, err := sqlite.NewWithDB(db, db, nil); err != nil {
t.Fatalf("replay migrations: %v", err)
}
var gotTimezone string
if err := db.GetContext(ctx, &gotTimezone,
`SELECT timezone FROM office_routine_triggers WHERE id = $1`, trigger.ID,
); err != nil {
t.Fatalf("read post-migration timezone: %v", err)
}
if gotTimezone != "UTC" {
t.Errorf("timezone after backfill = %q, want %q", gotTimezone, "UTC")
}
}
@@ -0,0 +1,206 @@
package sqlite_test
import (
"context"
"testing"
"github.com/jmoiron/sqlx"
_ "github.com/mattn/go-sqlite3"
settingsstore "github.com/kandev/kandev/internal/agent/settings/store"
"github.com/kandev/kandev/internal/office/models"
"github.com/kandev/kandev/internal/office/repository/sqlite"
)
// TestBackfillRoutineTriggerTimezones_LegacyEmptyTimezoneBecomesUTC reproduces
// a pre-migration row (a cron trigger created before the column default
// changed from empty to 'UTC') and asserts a migration replay backfills it,
// mirroring the table-rebuild replay pattern used elsewhere in this package
// (e.g. TestInitSchema_ExecutionProfileRoutingColumnsReplay).
func TestBackfillRoutineTriggerTimezones_LegacyEmptyTimezoneBecomesUTC(t *testing.T) {
db, err := sqlx.Open("sqlite3", ":memory:")
if err != nil {
t.Fatalf("open sqlite: %v", err)
}
db.SetMaxOpenConns(1)
t.Cleanup(func() { _ = db.Close() })
if _, _, err := settingsstore.Provide(db, db, nil); err != nil {
t.Fatalf("settings store init: %v", err)
}
repo, err := sqlite.NewWithDB(db, db, nil)
if err != nil {
t.Fatalf("first init: %v", err)
}
ctx := context.Background()
routine := &models.Routine{
WorkspaceID: "ws-1",
Name: "Legacy Cron",
TaskTemplate: "{}",
Status: "active",
ConcurrencyPolicy: "skip_if_active",
Variables: "{}",
}
if err := repo.CreateRoutine(ctx, routine); err != nil {
t.Fatalf("create routine: %v", err)
}
trigger := &models.RoutineTrigger{
RoutineID: routine.ID,
Kind: "cron",
CronExpression: "0 9 * * *",
Timezone: "UTC",
Enabled: true,
}
if err := repo.CreateRoutineTrigger(ctx, trigger); err != nil {
t.Fatalf("create trigger: %v", err)
}
// Simulate the legacy state the backfill exists to fix: a cron trigger
// persisted before the column default changed from '' to 'UTC'.
if _, err := db.Exec(`UPDATE office_routine_triggers SET timezone = '' WHERE id = ?`, trigger.ID); err != nil {
t.Fatalf("simulate legacy empty timezone: %v", err)
}
var preTimezone string
if err := db.Get(&preTimezone, `SELECT timezone FROM office_routine_triggers WHERE id = ?`, trigger.ID); err != nil {
t.Fatalf("read pre-migration timezone: %v", err)
}
if preTimezone != "" {
t.Fatalf("pre-migration timezone = %q, want empty string", preTimezone)
}
// Replay migrations on the same DB handle, as a restart would.
if _, err := sqlite.NewWithDB(db, db, nil); err != nil {
t.Fatalf("replay migrations: %v", err)
}
var gotTimezone string
if err := db.Get(&gotTimezone, `SELECT timezone FROM office_routine_triggers WHERE id = ?`, trigger.ID); err != nil {
t.Fatalf("read post-migration timezone: %v", err)
}
if gotTimezone != "UTC" {
t.Errorf("timezone after backfill = %q, want %q", gotTimezone, "UTC")
}
}
func TestBackfillRoutineTriggerTimezones_ReportsUpdateFailure(t *testing.T) {
db, err := sqlx.Open("sqlite3", ":memory:")
if err != nil {
t.Fatalf("open sqlite: %v", err)
}
db.SetMaxOpenConns(1)
t.Cleanup(func() { _ = db.Close() })
if _, _, err := settingsstore.Provide(db, db, nil); err != nil {
t.Fatalf("settings store init: %v", err)
}
repo, err := sqlite.NewWithDB(db, db, nil)
if err != nil {
t.Fatalf("first init: %v", err)
}
routine := &models.Routine{
WorkspaceID: "ws-1",
Name: "Legacy Cron Failure",
TaskTemplate: "{}",
Status: "active",
ConcurrencyPolicy: "skip_if_active",
Variables: "{}",
}
if err := repo.CreateRoutine(context.Background(), routine); err != nil {
t.Fatalf("create routine: %v", err)
}
trigger := &models.RoutineTrigger{
RoutineID: routine.ID,
Kind: "cron",
CronExpression: "0 9 * * *",
Timezone: "",
Enabled: true,
}
if err := repo.CreateRoutineTrigger(context.Background(), trigger); err != nil {
t.Fatalf("create trigger: %v", err)
}
if _, err := db.Exec(`UPDATE office_routine_triggers SET timezone = '' WHERE id = ?`, trigger.ID); err != nil {
t.Fatalf("simulate legacy empty timezone: %v", err)
}
if _, err := db.Exec(`
CREATE TRIGGER fail_routine_timezone_backfill
BEFORE UPDATE OF timezone ON office_routine_triggers
WHEN OLD.id = NEW.id AND OLD.timezone = '' AND NEW.timezone = 'UTC'
BEGIN
SELECT RAISE(ABORT, 'injected backfill failure');
END;
`); err != nil {
t.Fatalf("create failure trigger: %v", err)
}
if _, err := sqlite.NewWithDB(db, db, nil); err == nil {
t.Fatal("migration replay succeeded after timezone backfill failure")
}
}
// TestBackfillRoutineTriggerTimezones_LeavesNonEmptyAndNonCronAlone verifies
// the backfill only touches cron triggers with an empty timezone, not a
// deliberately-set timezone or a non-cron (webhook/manual) trigger kind.
func TestBackfillRoutineTriggerTimezones_LeavesNonEmptyAndNonCronAlone(t *testing.T) {
db, err := sqlx.Open("sqlite3", ":memory:")
if err != nil {
t.Fatalf("open sqlite: %v", err)
}
db.SetMaxOpenConns(1)
t.Cleanup(func() { _ = db.Close() })
if _, _, err := settingsstore.Provide(db, db, nil); err != nil {
t.Fatalf("settings store init: %v", err)
}
repo, err := sqlite.NewWithDB(db, db, nil)
if err != nil {
t.Fatalf("first init: %v", err)
}
ctx := context.Background()
routine := &models.Routine{
WorkspaceID: "ws-1",
Name: "Mixed Triggers",
TaskTemplate: "{}",
Status: "active",
ConcurrencyPolicy: "skip_if_active",
Variables: "{}",
}
if err := repo.CreateRoutine(ctx, routine); err != nil {
t.Fatalf("create routine: %v", err)
}
cronExplicit := &models.RoutineTrigger{RoutineID: routine.ID, Kind: "cron", CronExpression: "0 9 * * *", Timezone: "America/New_York", Enabled: true}
if err := repo.CreateRoutineTrigger(ctx, cronExplicit); err != nil {
t.Fatalf("create explicit-timezone trigger: %v", err)
}
webhook := &models.RoutineTrigger{RoutineID: routine.ID, Kind: "webhook", PublicID: "wh-1", SigningMode: "none", Enabled: true}
if err := repo.CreateRoutineTrigger(ctx, webhook); err != nil {
t.Fatalf("create webhook trigger: %v", err)
}
if _, err := db.Exec(`UPDATE office_routine_triggers SET timezone = '' WHERE id = ?`, webhook.ID); err != nil {
t.Fatalf("simulate empty timezone on webhook trigger: %v", err)
}
if _, err := sqlite.NewWithDB(db, db, nil); err != nil {
t.Fatalf("replay migrations: %v", err)
}
var cronTZ, webhookTZ string
if err := db.Get(&cronTZ, `SELECT timezone FROM office_routine_triggers WHERE id = ?`, cronExplicit.ID); err != nil {
t.Fatalf("read cron timezone: %v", err)
}
if cronTZ != "America/New_York" {
t.Errorf("explicit cron timezone = %q, want unchanged %q", cronTZ, "America/New_York")
}
if err := db.Get(&webhookTZ, `SELECT timezone FROM office_routine_triggers WHERE id = ?`, webhook.ID); err != nil {
t.Fatalf("read webhook timezone: %v", err)
}
if webhookTZ != "" {
t.Errorf("webhook timezone = %q, want unchanged empty string (backfill is cron-only)", webhookTZ)
}
}
@@ -31,7 +31,7 @@ func (r *Repository) CreateRoutineTrigger(ctx context.Context, t *models.Routine
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
`), t.ID, t.RoutineID, t.Kind, t.CronExpression, t.Timezone,
t.PublicID, t.SigningMode, t.Secret, t.NextRunAt, t.LastFiredAt,
t.Enabled, t.CreatedAt, t.UpdatedAt)
boolToInt(t.Enabled), t.CreatedAt, t.UpdatedAt)
return err
}
@@ -5,6 +5,7 @@ import (
"crypto/sha256"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
@@ -183,7 +184,11 @@ func (h *Handler) createTrigger(c *gin.Context) {
Enabled: true,
}
if err := h.svc.CreateRoutineTrigger(c.Request.Context(), trigger); err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
status := http.StatusInternalServerError
if errors.Is(err, ErrInvalidTrigger) {
status = http.StatusBadRequest
}
c.JSON(status, gin.H{"error": err.Error()})
return
}
trigger.Secret = "" // redact before sending response
@@ -0,0 +1,80 @@
package routines_test
import (
"encoding/json"
"net/http"
"net/http/httptest"
"strings"
"testing"
"github.com/gin-gonic/gin"
"github.com/kandev/kandev/internal/office/routines"
)
// newTestRouter mounts routine routes over a fresh in-memory-SQLite-backed
// RoutineService.
func newTestRouter(t *testing.T) *gin.Engine {
t.Helper()
gin.SetMode(gin.TestMode)
svc := newTestRoutineService(t)
router := gin.New()
routines.RegisterRoutes(router.Group("/api/v1"), routines.NewHandler(svc))
return router
}
func doRequest(t *testing.T, router *gin.Engine, method, path, body string) *httptest.ResponseRecorder {
t.Helper()
req := httptest.NewRequest(method, path, strings.NewReader(body))
req.Header.Set("Content-Type", "application/json")
rec := httptest.NewRecorder()
router.ServeHTTP(rec, req)
return rec
}
func createTestRoutineViaHTTP(t *testing.T, router *gin.Engine) string {
t.Helper()
rec := doRequest(t, router, http.MethodPost, "/api/v1/workspaces/ws-1/routines", `{
"name": "r1",
"task_template": "{}"
}`)
if rec.Code != http.StatusCreated {
t.Fatalf("create routine status = %d, body = %s", rec.Code, rec.Body.String())
}
var resp routines.RoutineResponse
if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil {
t.Fatalf("decode routine response: %v", err)
}
return resp.Routine.ID
}
// TestCreateTrigger_InvalidCronExpression_Returns400 verifies a validation
// failure (unsatisfiable cron expression) is a client error, not a server
// error.
func TestCreateTrigger_InvalidCronExpression_Returns400(t *testing.T) {
router := newTestRouter(t)
routineID := createTestRoutineViaHTTP(t, router)
rec := doRequest(t, router, http.MethodPost, "/api/v1/routines/"+routineID+"/triggers", `{
"kind": "cron",
"cron_expression": "0 0 30 2 *"
}`)
if rec.Code != http.StatusBadRequest {
t.Fatalf("status = %d, want %d; body = %s", rec.Code, http.StatusBadRequest, rec.Body.String())
}
}
// TestCreateTrigger_ValidCronExpression_Returns201 is the control case: a
// well-formed cron trigger still creates successfully.
func TestCreateTrigger_ValidCronExpression_Returns201(t *testing.T) {
router := newTestRouter(t)
routineID := createTestRoutineViaHTTP(t, router)
rec := doRequest(t, router, http.MethodPost, "/api/v1/routines/"+routineID+"/triggers", `{
"kind": "cron",
"cron_expression": "0 9 * * *"
}`)
if rec.Code != http.StatusCreated {
t.Fatalf("status = %d, want %d; body = %s", rec.Code, http.StatusCreated, rec.Body.String())
}
}
@@ -27,6 +27,10 @@ import (
// imports office/repository/sqlite directly (see routineWakeupAdapter).
var ErrWakeupAlreadyRequested = errors.New("wakeup request already requested")
// ErrInvalidTrigger indicates a trigger create request that failed
// validation (a client error, not a server failure).
var ErrInvalidTrigger = errors.New("invalid routine trigger")
// Repository is the persistence interface required by RoutineService.
type Repository interface {
CreateRoutine(ctx context.Context, routine *Routine) error
@@ -329,11 +333,20 @@ func (s *RoutineService) ListRoutinesFromConfig(ctx context.Context, workspaceID
// -- Trigger management --
// CreateRoutineTrigger creates a trigger and computes next_run_at for cron.
// A cron trigger requires a satisfiable expression: an empty expression, or
// one that can never fire, is rejected here rather than becoming a silent
// no-op or a wrong daily fallback at tick time.
func (s *RoutineService) CreateRoutineTrigger(ctx context.Context, t *RoutineTrigger) error {
if t.Kind == "cron" && t.CronExpression != "" {
if t.Timezone == "" {
t.Timezone = "UTC"
}
if t.Kind == "cron" {
if t.CronExpression == "" {
return fmt.Errorf("%w: cron trigger requires a cron_expression", ErrInvalidTrigger)
}
next, err := shared.NextCronTime(t.CronExpression, t.Timezone, time.Now().UTC())
if err != nil {
return fmt.Errorf("invalid cron expression: %w", err)
return fmt.Errorf("%w: invalid cron expression: %v", ErrInvalidTrigger, err)
}
t.NextRunAt = &next
}
@@ -402,11 +415,25 @@ func (s *RoutineService) processCronTrigger(ctx context.Context, trigger *Routin
// expressions (no fixed interval — we walk NextCronTime).
runCount, advanceTo, err := computeRoutineMissed(trigger, routine, now)
if err != nil {
s.logger.Warn("compute routine catch-up failed",
if errors.Is(err, shared.ErrUnsatisfiableCron) {
// The expression can never fire again. ClaimTrigger already
// cleared next_run_at; leave it cleared rather than re-arming,
// which would dispatch nothing but retry (and fail) forever.
s.logger.Error("cron expression unsatisfiable; trigger permanently disarmed",
zap.String("trigger_id", trigger.ID), zap.Error(err))
return err
}
// Any other failure (e.g. the timezone database is temporarily
// unavailable) is presumed recoverable: re-arm to the original due
// time so the next tick retries instead of leaving the trigger
// disarmed forever once the underlying issue clears.
if rearmErr := s.repo.UpdateTriggerNextRun(ctx, trigger.ID, trigger.NextRunAt); rearmErr != nil {
s.logger.Warn("re-arm trigger after recoverable catch-up failure failed",
zap.String("trigger_id", trigger.ID), zap.Error(rearmErr))
}
s.logger.Warn("compute routine catch-up failed; will retry next tick",
zap.String("trigger_id", trigger.ID), zap.Error(err))
// Fall through with sane defaults: fire once, advance one tick.
runCount = 1
advanceTo = now
return err
}
if err := s.repo.UpdateTriggerNextRun(ctx, trigger.ID, &advanceTo); err != nil {
s.logger.Warn("update trigger next_run_at failed",
@@ -0,0 +1,169 @@
package routines_test
import (
"context"
"testing"
"time"
"github.com/jmoiron/sqlx"
_ "github.com/mattn/go-sqlite3"
"github.com/kandev/kandev/internal/common/logger"
"github.com/kandev/kandev/internal/office/models"
"github.com/kandev/kandev/internal/office/repository/sqlite"
"github.com/kandev/kandev/internal/office/routines"
)
// TestTickScheduledTriggers_UnsatisfiableExpression_DisarmsInsteadOfLooping
// is a regression test for a legacy trigger row whose cron expression can
// never fire (e.g. persisted before create-time validation existed, or
// hand-edited). On a catch-up computation failure the trigger must not
// dispatch and must not re-arm next_run_at to "now": ClaimTrigger already
// clears next_run_at when claiming, and re-arming it to "now" makes the
// trigger due again on the very next 30s cron tick, dispatching a run
// forever.
func TestTickScheduledTriggers_UnsatisfiableExpression_DisarmsInsteadOfLooping(t *testing.T) {
db, err := sqlx.Open("sqlite3", ":memory:")
if err != nil {
t.Fatalf("open sqlite: %v", err)
}
t.Cleanup(func() { _ = db.Close() })
repo, err := sqlite.NewWithDB(db, db, nil)
if err != nil {
t.Fatalf("new repo: %v", err)
}
svc := routines.NewRoutineService(repo, logger.Default(), &noopActivity{})
ctx := context.Background()
routine := &models.Routine{
WorkspaceID: "ws-1",
Name: "Legacy unsatisfiable",
TaskTemplate: "", // lightweight
AssigneeAgentProfileID: "agent-1",
Status: "active",
ConcurrencyPolicy: models.ConcurrencyPolicyAlwaysCreate,
}
if err := svc.CreateRoutine(ctx, routine); err != nil {
t.Fatalf("create routine: %v", err)
}
// Bypass service-level create-time validation (routines.ErrInvalidTrigger)
// by writing straight through the repository, simulating a row that was
// persisted before that validation existed.
due := time.Now().UTC().Add(-time.Minute)
trigger := &models.RoutineTrigger{
RoutineID: routine.ID,
Kind: "cron",
CronExpression: "0 0 30 2 *", // February 30th: never matches.
Timezone: "UTC",
Enabled: true,
NextRunAt: &due,
}
if err := repo.CreateRoutineTrigger(ctx, trigger); err != nil {
t.Fatalf("create trigger (bypassing validation): %v", err)
}
enq := &fakeWakeupEnqueuer{}
svc.SetWakeupEnqueuer(enq)
if err := svc.TickScheduledTriggers(ctx, time.Now().UTC()); err != nil {
t.Fatalf("tick scheduled triggers: %v", err)
}
if len(enq.created) != 0 {
t.Fatalf("expected no dispatch for an unsatisfiable trigger, got %d", len(enq.created))
}
triggers, err := repo.ListTriggersByRoutineID(ctx, routine.ID)
if err != nil {
t.Fatalf("list triggers: %v", err)
}
if len(triggers) != 1 {
t.Fatalf("expected 1 trigger, got %d", len(triggers))
}
if triggers[0].NextRunAt != nil {
t.Fatalf("next_run_at = %v, want nil (trigger must stay disarmed, not re-armed to now)", *triggers[0].NextRunAt)
}
// A second tick must not dispatch either — proves this isn't a
// re-arm-to-now loop that would fire again on the very next 30s tick.
if err := svc.TickScheduledTriggers(ctx, time.Now().UTC().Add(time.Minute)); err != nil {
t.Fatalf("second tick scheduled triggers: %v", err)
}
if len(enq.created) != 0 {
t.Fatalf("expected still no dispatch after second tick, got %d", len(enq.created))
}
}
// TestTickScheduledTriggers_RecoverableCatchUpFailure_RearmsForRetry is a
// regression test for a catch-up failure that is NOT expression
// unsatisfiability (e.g. the timezone database is transiently unavailable).
// Unlike the unsatisfiable case, this must re-arm next_run_at to the
// original due time so the next tick retries once the underlying issue
// clears, instead of leaving the trigger permanently disarmed.
func TestTickScheduledTriggers_RecoverableCatchUpFailure_RearmsForRetry(t *testing.T) {
db, err := sqlx.Open("sqlite3", ":memory:")
if err != nil {
t.Fatalf("open sqlite: %v", err)
}
t.Cleanup(func() { _ = db.Close() })
repo, err := sqlite.NewWithDB(db, db, nil)
if err != nil {
t.Fatalf("new repo: %v", err)
}
svc := routines.NewRoutineService(repo, logger.Default(), &noopActivity{})
ctx := context.Background()
routine := &models.Routine{
WorkspaceID: "ws-1",
Name: "Bad timezone",
TaskTemplate: "",
AssigneeAgentProfileID: "agent-1",
Status: "active",
ConcurrencyPolicy: models.ConcurrencyPolicyAlwaysCreate,
}
if err := svc.CreateRoutine(ctx, routine); err != nil {
t.Fatalf("create routine: %v", err)
}
// Bypass service-level create-time validation by writing straight
// through the repository, simulating a timezone that was valid at
// create time but whose lookup now fails (e.g. tzdata unavailable).
due := time.Now().UTC().Add(-time.Minute)
trigger := &models.RoutineTrigger{
RoutineID: routine.ID,
Kind: "cron",
CronExpression: "*/5 * * * *",
Timezone: "Not/AZone",
Enabled: true,
NextRunAt: &due,
}
if err := repo.CreateRoutineTrigger(ctx, trigger); err != nil {
t.Fatalf("create trigger (bypassing validation): %v", err)
}
enq := &fakeWakeupEnqueuer{}
svc.SetWakeupEnqueuer(enq)
if err := svc.TickScheduledTriggers(ctx, time.Now().UTC()); err != nil {
t.Fatalf("tick scheduled triggers: %v", err)
}
if len(enq.created) != 0 {
t.Fatalf("expected no dispatch on a recoverable catch-up failure, got %d", len(enq.created))
}
triggers, err := repo.ListTriggersByRoutineID(ctx, routine.ID)
if err != nil {
t.Fatalf("list triggers: %v", err)
}
if len(triggers) != 1 {
t.Fatalf("expected 1 trigger, got %d", len(triggers))
}
if triggers[0].NextRunAt == nil {
t.Fatalf("next_run_at = nil, want re-armed to the original due time so the trigger retries")
}
if !triggers[0].NextRunAt.Equal(due) {
t.Fatalf("next_run_at = %v, want re-armed to original due time %v", *triggers[0].NextRunAt, due)
}
}
@@ -0,0 +1,81 @@
package routines_test
import (
"context"
"errors"
"testing"
"github.com/kandev/kandev/internal/office/routines"
)
// TestCreateRoutineTrigger_EmptyCronExpression_Rejected verifies a cron
// trigger with no expression is rejected at create time instead of
// persisting as a trigger that silently never fires.
func TestCreateRoutineTrigger_EmptyCronExpression_Rejected(t *testing.T) {
svc := newTestRoutineService(t)
routine := createTestRoutine(t, svc, "empty-cron", "skip_if_active")
trigger := &routines.RoutineTrigger{RoutineID: routine.ID, Kind: "cron", CronExpression: ""}
err := svc.CreateRoutineTrigger(context.Background(), trigger)
if err == nil {
t.Fatal("expected error for empty cron expression")
}
if !errors.Is(err, routines.ErrInvalidTrigger) {
t.Errorf("expected ErrInvalidTrigger, got %v", err)
}
}
// TestCreateRoutineTrigger_UnsatisfiableCron_Rejected verifies an impossible
// expression (Feb 30th) is rejected at create time instead of becoming a
// silent daily-at-midnight job.
func TestCreateRoutineTrigger_UnsatisfiableCron_Rejected(t *testing.T) {
svc := newTestRoutineService(t)
routine := createTestRoutine(t, svc, "unsatisfiable-cron", "skip_if_active")
trigger := &routines.RoutineTrigger{RoutineID: routine.ID, Kind: "cron", CronExpression: "0 0 30 2 *"}
err := svc.CreateRoutineTrigger(context.Background(), trigger)
if err == nil {
t.Fatal("expected error for unsatisfiable cron expression")
}
if !errors.Is(err, routines.ErrInvalidTrigger) {
t.Errorf("expected ErrInvalidTrigger, got %v", err)
}
}
// TestCreateRoutineTrigger_EmptyTimezone_NormalizedToUTC verifies an empty
// timezone is made explicit at write time rather than persisted as ”.
func TestCreateRoutineTrigger_EmptyTimezone_NormalizedToUTC(t *testing.T) {
svc := newTestRoutineService(t)
routine := createTestRoutine(t, svc, "empty-tz", "skip_if_active")
trigger := &routines.RoutineTrigger{RoutineID: routine.ID, Kind: "cron", CronExpression: "0 9 * * *"}
if err := svc.CreateRoutineTrigger(context.Background(), trigger); err != nil {
t.Fatalf("create trigger: %v", err)
}
if trigger.Timezone != "UTC" {
t.Errorf("Timezone = %q, want %q", trigger.Timezone, "UTC")
}
triggers, err := svc.ListRoutineTriggers(context.Background(), routine.ID)
if err != nil {
t.Fatalf("list triggers: %v", err)
}
if len(triggers) != 1 || triggers[0].Timezone != "UTC" {
t.Fatalf("persisted triggers = %+v, want one trigger with Timezone=UTC", triggers)
}
}
// TestCreateRoutineTrigger_ValidCron_Succeeds is the control case: a
// well-formed cron trigger still creates successfully.
func TestCreateRoutineTrigger_ValidCron_Succeeds(t *testing.T) {
svc := newTestRoutineService(t)
routine := createTestRoutine(t, svc, "valid-cron", "skip_if_active")
trigger := &routines.RoutineTrigger{RoutineID: routine.ID, Kind: "cron", CronExpression: "*/5 * * * *"}
if err := svc.CreateRoutineTrigger(context.Background(), trigger); err != nil {
t.Fatalf("create trigger: %v", err)
}
if trigger.NextRunAt == nil {
t.Error("expected NextRunAt to be set")
}
}
+212 -216
View File
@@ -1,251 +1,247 @@
package shared
import (
"errors"
"fmt"
"strconv"
"strings"
"time"
"github.com/robfig/cron/v3"
)
// NextCronTime computes the next fire time after `after` for the given cron expression.
// Supports standard 5-field cron: minute hour day-of-month month day-of-week.
// Timezone is optional; defaults to UTC.
// ErrUnsatisfiableCron indicates a syntactically valid but impossible cron
// expression (e.g. "0 0 30 2 *", February 30th) that can never match a
// wall-clock date.
var ErrUnsatisfiableCron = errors.New("cron expression can never fire")
// cronParser accepts the standard 5-field cron syntax: minute hour
// day-of-month month day-of-week. No descriptors (@daily, @every) - Office's
// contract is strictly 5 fields.
var cronParser = cron.NewParser(
cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow,
)
// NextCronTime computes the next fire time strictly after `after` for the
// given 5-field cron expression, in the given timezone (empty means UTC).
//
// Day-of-month and day-of-week are ORed when both are restricted, matching
// crontab(5): "0 0 13 * 5" means the 13th of the month OR any Friday, not
// only Friday the 13th.
//
// DST policy: a wall-clock slot fires at most once. A slot that does not
// exist (spring-forward gap) is skipped. A slot that occurs twice (fall-back
// repeated hour) fires only on its first occurrence.
func NextCronTime(expression, timezone string, after time.Time) (time.Time, error) {
fields := strings.Fields(expression)
if len(fields) != 5 {
return time.Time{}, fmt.Errorf("expected 5 cron fields, got %d", len(fields))
}
loc := time.UTC
if timezone != "" {
var err error
loc, err = time.LoadLocation(timezone)
if err != nil {
return time.Time{}, fmt.Errorf("invalid timezone %q: %w", timezone, err)
}
}
spec, err := parseCronSpec(fields)
loc, err := resolveLocation(timezone)
if err != nil {
return time.Time{}, err
}
return findNextMatch(spec, after.In(loc)), nil
}
type cronSpec struct {
minutes []int
hours []int
daysOfMonth []int
months []int
daysOfWeek []int
}
// dayOfWeekNames maps abbreviated day names to their numeric cron value.
// Case-insensitive; lookup is performed after lowercasing the token.
var dayOfWeekNames = map[string]int{
"sun": 0, "mon": 1, "tue": 2, "wed": 3, "thu": 4, "fri": 5, "sat": 6,
}
// monthNames maps abbreviated month names to their numeric cron value.
// Case-insensitive; lookup is performed after lowercasing the token.
var monthNames = map[string]int{
"jan": 1, "feb": 2, "mar": 3, "apr": 4, "may": 5, "jun": 6,
"jul": 7, "aug": 8, "sep": 9, "oct": 10, "nov": 11, "dec": 12,
}
func parseCronSpec(fields []string) (*cronSpec, error) {
minutes, err := parseCronField(fields[0], 0, 59, nil)
trimmed := strings.TrimSpace(expression)
if len(strings.Fields(trimmed)) != 5 {
return time.Time{}, fmt.Errorf("parse cron expression: %q: must be exactly 5 whitespace-separated fields (minute hour day-of-month month day-of-week); descriptors and TZ/CRON_TZ prefixes are not supported", expression)
}
schedule, err := cronParser.Parse(trimmed)
if err != nil {
return nil, fmt.Errorf("minute field: %w", err)
return time.Time{}, fmt.Errorf("parse cron expression: %w", err)
}
hours, err := parseCronField(fields[1], 0, 23, nil)
if err != nil {
return nil, fmt.Errorf("hour field: %w", err)
}
dom, err := parseCronField(fields[2], 1, 31, nil)
if err != nil {
return nil, fmt.Errorf("day-of-month field: %w", err)
}
months, err := parseCronField(fields[3], 1, 12, monthNames)
if err != nil {
return nil, fmt.Errorf("month field: %w", err)
}
dow, err := parseCronField(fields[4], 0, 6, dayOfWeekNames)
if err != nil {
return nil, fmt.Errorf("day-of-week field: %w", err)
}
return &cronSpec{minutes, hours, dom, months, dow}, nil
}
// parseCronField parses a single cron field (supports *, N, N-M, */N, N-M/N, comma lists).
// If names is non-nil, alphabetic tokens are resolved to numbers via the map
// before numeric parsing (e.g. MON->1, JAN->1). Lookup is case-insensitive.
func parseCronField(field string, min, max int, names map[string]int) ([]int, error) {
var result []int
for _, part := range strings.Split(field, ",") {
vals, err := parseCronPart(part, min, max, names)
if err != nil {
return nil, err
}
result = append(result, vals...)
}
return dedupSort(result), nil
}
func parseCronPart(part string, min, max int, names map[string]int) ([]int, error) {
step, base, err := extractStep(part)
if err != nil {
return nil, err
}
resolved, err := resolveNames(base, names)
if err != nil {
return nil, err
}
lo, hi, err := parseRange(resolved, min, max)
if err != nil {
return nil, err
}
if lo < min || hi > max || lo > hi {
return nil, fmt.Errorf("value out of range [%d-%d] in %q", min, max, part)
}
var vals []int
for i := lo; i <= hi; i += step {
vals = append(vals, i)
}
return vals, nil
}
// resolveNames translates any alphabetic tokens in a cron part (e.g. "MON",
// "mon-fri") into their numeric equivalents using the provided name map.
// Wildcards are passed through. If names is nil and the part contains
// non-numeric tokens, an error is returned by the downstream numeric parser.
func resolveNames(part string, names map[string]int) (string, error) {
if part == "*" || !containsAlpha(part) {
return part, nil
}
if names == nil {
return "", fmt.Errorf("alphabetic name %q not allowed in this field", part)
}
if idx := strings.Index(part, "-"); idx >= 0 {
lo, err := resolveToken(part[:idx], names)
if err != nil {
return "", err
}
hi, err := resolveToken(part[idx+1:], names)
if err != nil {
return "", err
}
return fmt.Sprintf("%d-%d", lo, hi), nil
}
val, err := resolveToken(part, names)
if err != nil {
return "", err
}
return strconv.Itoa(val), nil
}
func resolveToken(tok string, names map[string]int) (int, error) {
if !containsAlpha(tok) {
v, err := strconv.Atoi(tok)
if err != nil {
return 0, fmt.Errorf("invalid value %q", tok)
}
return v, nil
}
v, ok := names[strings.ToLower(tok)]
specSchedule, ok := schedule.(*cron.SpecSchedule)
if !ok {
return 0, fmt.Errorf("unknown name %q", tok)
// cronParser is configured with only the 5 standard fields (no
// descriptors), so Parse always returns *SpecSchedule for a
// well-formed 5-field expression. This would only trip if
// robfig/cron's internal type changed — fail loudly rather than
// silently degrading matchesWallClock to "everything matches".
return time.Time{}, fmt.Errorf("internal: unexpected schedule type %T", schedule)
}
return v, nil
start := after.In(loc)
candidate := schedule.Next(start)
if candidate.IsZero() {
return time.Time{}, fmt.Errorf("%w: %q", ErrUnsatisfiableCron, expression)
}
for isAmbiguousFallBack(candidate) || !matchesWallClock(specSchedule, candidate) {
candidate = schedule.Next(candidate)
if candidate.IsZero() {
return time.Time{}, fmt.Errorf("%w: %q", ErrUnsatisfiableCron, expression)
}
}
if earlier, ok := findEarlierMatchAcrossSubHourTransition(specSchedule, loc, start, candidate); ok {
candidate = earlier
}
return candidate.UTC(), nil
}
func containsAlpha(s string) bool {
for _, r := range s {
if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') {
// findEarlierMatchAcrossSubHourTransition recovers a fire that
// schedule.Next skipped over because its hour-advancing loop steps by an
// absolute 1h and desynchronizes from a zone transition whose offset delta
// is not a whole hour (only Australia/Lord_Howe, +10:30<->+11:00, does this
// among IANA zones): the loop can jump straight past a day that has a
// genuinely matching slot. matchesWallClock cannot recover this on its own
// because it only validates candidates schedule.Next actually returns.
//
// This rescans only the local days around each affected transition at minute
// granularity, so a sparse schedule cannot turn the shared scheduler tick
// into a multi-year scan.
func findEarlierMatchAcrossSubHourTransition(spec *cron.SpecSchedule, loc *time.Location, after, candidate time.Time) (time.Time, bool) {
if spec == nil {
return time.Time{}, false
}
var earlier time.Time
found := walkZoneTransitions(loc, after, candidate, func(transition time.Time) bool {
delta := zoneOffsetAt(transition) - zoneOffsetAt(transition.Add(-time.Second))
if delta < 0 {
delta = -delta
}
if delta%3600 == 0 {
return false
}
windowStart, windowEnd := transitionDayWindow(loc, transition)
t := after.Truncate(time.Minute)
if !t.After(after) {
t = t.Add(time.Minute)
}
if t.Before(windowStart) {
t = windowStart
}
if candidate.Before(windowEnd) {
windowEnd = candidate
}
for t.Before(windowEnd) {
if !isAmbiguousFallBack(t) && matchesWallClock(spec, t) {
earlier = t
return true
}
t = t.Add(time.Minute)
}
return false
})
return earlier, found
}
const maxZoneTransitionHops = 512
// walkZoneTransitions visits each transition strictly between after and
// candidate. Some TZif files use a POSIX extension whose ZoneBounds result
// can stop advancing at a leap-year boundary, so a non-progressing result is
// advanced to the next local year before the next lookup. The hop limit is a
// final guard against malformed or unexpectedly long timezone data.
func walkZoneTransitions(loc *time.Location, after, candidate time.Time, visit func(time.Time) bool) bool {
if loc == nil || visit == nil || !after.Before(candidate) {
return false
}
t := after.In(loc)
for hops := 0; hops < maxZoneTransitionHops; hops++ {
_, transition := t.ZoneBounds()
if transition.IsZero() || !transition.Before(candidate) {
return false
}
if !transition.After(t) {
nextYear := time.Date(t.Year()+1, time.January, 1, 0, 0, 0, 0, loc)
if !nextYear.After(t) {
return false
}
t = nextYear
continue
}
if visit(transition) {
return true
}
t = transition
}
return false
}
func extractStep(part string) (int, string, error) {
idx := strings.Index(part, "/")
if idx < 0 {
return 1, part, nil
}
step, err := strconv.Atoi(part[idx+1:])
if err != nil || step <= 0 {
return 0, "", fmt.Errorf("invalid step in %q", part)
}
return step, part[:idx], nil
func transitionDayWindow(loc *time.Location, transition time.Time) (time.Time, time.Time) {
local := transition.In(loc)
start := time.Date(local.Year(), local.Month(), local.Day()-1, 0, 0, 0, 0, loc)
end := time.Date(local.Year(), local.Month(), local.Day()+2, 0, 0, 0, 0, loc)
return start, end
}
func parseRange(part string, min, max int) (int, int, error) {
if part == "*" {
return min, max, nil
func zoneOffsetAt(t time.Time) int {
_, offset := t.Zone()
return offset
}
// matchesWallClock reports whether candidate's local wall-clock fields
// actually satisfy spec. In a sub-hour DST zone (e.g. Australia/Lord_Howe,
// +10:30/+11:00), robfig's minute-increment loop advances in absolute time:
// crossing a spring-forward gap can shift the hour by 30 minutes without
// re-entering the hour-matching loop, so the returned instant can satisfy
// the minute bitmask under a different, unmatched hour. NextCronTime's own
// type-assertion guard means spec is never nil on that call path; the nil
// check here is defense against a future caller.
func matchesWallClock(spec *cron.SpecSchedule, candidate time.Time) bool {
if spec == nil {
return true
}
if idx := strings.Index(part, "-"); idx >= 0 {
lo, err := strconv.Atoi(part[:idx])
if err != nil {
return 0, 0, fmt.Errorf("invalid range in %q", part)
}
hi, err := strconv.Atoi(part[idx+1:])
if err != nil {
return 0, 0, fmt.Errorf("invalid range in %q", part)
}
return lo, hi, nil
if 1<<uint(candidate.Month())&spec.Month == 0 {
return false
}
val, err := strconv.Atoi(part)
if 1<<uint(candidate.Hour())&spec.Hour == 0 {
return false
}
if 1<<uint(candidate.Minute())&spec.Minute == 0 {
return false
}
if 1<<uint(candidate.Second())&spec.Second == 0 {
return false
}
return dayMatches(spec, candidate)
}
// dayMatches mirrors robfig/cron's SpecSchedule.dayMatches (unexported):
// when at least one of day-of-month or day-of-week is unrestricted (carries
// starBit), the fields are ANDed (the wildcard side is always true, so only
// the restricted side matters). When both are restricted, they are ORed,
// per crontab(5).
func dayMatches(spec *cron.SpecSchedule, t time.Time) bool {
const starBit = 1 << 63
domMatch := 1<<uint(t.Day())&spec.Dom > 0
dowMatch := 1<<uint(t.Weekday())&spec.Dow > 0
if spec.Dom&starBit > 0 || spec.Dow&starBit > 0 {
return domMatch && dowMatch
}
return domMatch || dowMatch
}
// resolveLocation maps an empty timezone to UTC, matching
// internal/automation/scheduler.go's nextCronFire.
func resolveLocation(timezone string) (*time.Location, error) {
if timezone == "" {
return time.UTC, nil
}
loc, err := time.LoadLocation(timezone)
if err != nil {
return 0, 0, fmt.Errorf("invalid value %q", part)
return nil, fmt.Errorf("invalid timezone %q: %w", timezone, err)
}
return val, val, nil
return loc, nil
}
func dedupSort(vals []int) []int {
seen := make(map[int]bool, len(vals))
var result []int
for _, v := range vals {
if !seen[v] {
seen[v] = true
result = append(result, v)
}
// isAmbiguousFallBack reports whether candidate is the second occurrence of
// a local wall-clock time made ambiguous by a DST fall-back transition.
//
// time.Date's disambiguation of ambiguous wall-clock fields is documented as
// implementation-defined ("the choice of time zone, and therefore the time,
// is not guaranteed"), and in practice resolves to different occurrences in
// different zone families (e.g. it picks the earlier instant in
// America/New_York but the later one in zones with a UTC+0 winter offset
// such as Europe/London). This instead reasons from candidate's own zone
// transition: candidate's period starts at the most recent offset change;
// if that change was a fall-back (offset decreased), the first
// offsetDelta-wide slice of the new period repeats wall-clock times already
// seen under the old offset.
func isAmbiguousFallBack(candidate time.Time) bool {
start, _ := candidate.ZoneBounds()
if start.IsZero() {
return false
}
// Simple insertion sort for small slices.
for i := 1; i < len(result); i++ {
for j := i; j > 0 && result[j] < result[j-1]; j-- {
result[j], result[j-1] = result[j-1], result[j]
}
_, currentOffset := candidate.Zone()
_, priorOffset := start.Add(-time.Second).Zone()
if priorOffset <= currentOffset {
return false // not a fall-back transition (spring-forward or no change)
}
return result
}
func findNextMatch(spec *cronSpec, after time.Time) time.Time {
// Start from the next minute after `after`.
t := after.Truncate(time.Minute).Add(time.Minute)
// Search up to 366 days ahead.
deadline := after.Add(366 * 24 * time.Hour)
for t.Before(deadline) {
if matchesSpec(spec, t) {
return t.UTC()
}
t = t.Add(time.Minute)
}
// Fallback: return 24h from now.
return after.Add(24 * time.Hour).UTC()
}
func matchesSpec(spec *cronSpec, t time.Time) bool {
return contains(spec.minutes, t.Minute()) &&
contains(spec.hours, t.Hour()) &&
contains(spec.daysOfMonth, t.Day()) &&
contains(spec.months, int(t.Month())) &&
contains(spec.daysOfWeek, int(t.Weekday()))
}
func contains(vals []int, v int) bool {
for _, val := range vals {
if val == v {
return true
}
}
return false
repeatedWindow := time.Duration(priorOffset-currentOffset) * time.Second
return candidate.Before(start.Add(repeatedWindow))
}
@@ -0,0 +1,32 @@
package shared
import (
"testing"
"time"
)
func TestNextCronTime_PastLeapYearPOSIXExtensionTerminates(t *testing.T) {
const expression = "0 12 1 1 *"
after := time.Date(2040, time.January, 2, 0, 0, 0, 0, time.UTC)
want := time.Date(2041, time.January, 1, 17, 0, 0, 0, time.UTC)
done := make(chan struct{})
var got time.Time
var err error
go func() {
got, err = NextCronTime(expression, "America/New_York", after)
close(done)
}()
select {
case <-done:
if err != nil {
t.Fatalf("NextCronTime returned error: %v", err)
}
if !got.Equal(want) {
t.Fatalf("NextCronTime = %s, want %s", got, want)
}
case <-time.After(2 * time.Second):
t.Fatal("NextCronTime did not terminate across a POSIX timezone year boundary")
}
}
+378 -30
View File
@@ -1,6 +1,7 @@
package shared
import (
"errors"
"strings"
"testing"
"time"
@@ -77,36 +78,6 @@ func TestNextCronTime_StepExpression(t *testing.T) {
}
}
func TestParseCronField_Wildcard(t *testing.T) {
vals, err := parseCronField("*", 0, 59, nil)
if err != nil {
t.Fatal(err)
}
if len(vals) != 60 {
t.Errorf("expected 60 values, got %d", len(vals))
}
}
func TestParseCronField_Range(t *testing.T) {
vals, err := parseCronField("1-5", 0, 6, nil)
if err != nil {
t.Fatal(err)
}
if len(vals) != 5 {
t.Errorf("expected 5 values, got %d", len(vals))
}
}
func TestParseCronField_CommaList(t *testing.T) {
vals, err := parseCronField("1,3,5", 0, 6, nil)
if err != nil {
t.Fatal(err)
}
if len(vals) != 3 {
t.Errorf("expected 3 values, got %d", len(vals))
}
}
func TestNextCronTime_DayNames(t *testing.T) {
// 2026-04-25 is a Saturday; first MON-FRI window opens on Mon 2026-04-27.
after := time.Date(2026, 4, 25, 10, 0, 0, 0, time.UTC)
@@ -212,3 +183,380 @@ func TestNextCronTime_DayNameInMonthField_Rejected(t *testing.T) {
t.Fatal("expected error when day name MON appears in month field")
}
}
// TestNextCronTime_DomDowOred verifies crontab(5) semantics: when both
// day-of-month and day-of-week are restricted, a match on either field
// fires, not just a match on both ("Friday the 13th").
func TestNextCronTime_DomDowOred(t *testing.T) {
after := time.Date(2025, 12, 31, 23, 59, 0, 0, time.UTC)
want := []time.Time{
time.Date(2026, 1, 2, 0, 0, 0, 0, time.UTC), // Friday
time.Date(2026, 1, 9, 0, 0, 0, 0, time.UTC), // Friday
time.Date(2026, 1, 13, 0, 0, 0, 0, time.UTC), // the 13th (Tuesday)
time.Date(2026, 1, 16, 0, 0, 0, 0, time.UTC), // Friday
time.Date(2026, 1, 23, 0, 0, 0, 0, time.UTC), // Friday
}
cursor := after
for i, w := range want {
next, err := NextCronTime("0 0 13 * 5", "", cursor)
if err != nil {
t.Fatalf("fire %d: unexpected error: %v", i+1, err)
}
if !next.Equal(w) {
t.Errorf("fire %d: got %v, want %v", i+1, next, w)
}
cursor = next
}
}
// TestNextCronTime_DSTSpringForward_Skip verifies that a wall-clock slot
// which does not exist (spring-forward gap hour) is skipped, not fired at
// an adjusted time.
func TestNextCronTime_DSTSpringForward_Skip(t *testing.T) {
loc, err := time.LoadLocation("America/New_York")
if err != nil {
t.Fatalf("load location: %v", err)
}
// 2026-03-08 is spring-forward in America/New_York; 02:00 does not exist.
after := time.Date(2026, 3, 7, 12, 0, 0, 0, loc)
next, err := NextCronTime("0 2 * * *", "America/New_York", after)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
want := time.Date(2026, 3, 9, 2, 0, 0, 0, loc)
if !next.Equal(want) {
t.Errorf("got %v, want %v (2026-03-08 should be skipped)", next, want)
}
}
// TestNextCronTime_DSTFallBack_FiresOnce verifies that an ambiguous
// wall-clock slot (fall-back repeated hour) fires exactly once.
func TestNextCronTime_DSTFallBack_FiresOnce(t *testing.T) {
loc, err := time.LoadLocation("America/New_York")
if err != nil {
t.Fatalf("load location: %v", err)
}
// 2026-11-01 is fall-back in America/New_York; 01:30 occurs twice.
after := time.Date(2026, 10, 31, 12, 0, 0, 0, loc)
fire1, err := NextCronTime("30 1 * * *", "America/New_York", after)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
wantFire1 := time.Date(2026, 11, 1, 1, 30, 0, 0, loc)
if !fire1.Equal(wantFire1) {
t.Fatalf("fire1: got %v, want %v", fire1, wantFire1)
}
fire2, err := NextCronTime("30 1 * * *", "America/New_York", fire1)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
wantFire2 := time.Date(2026, 11, 2, 1, 30, 0, 0, loc)
if !fire2.Equal(wantFire2) {
t.Errorf("fire2: got %v, want %v (second 01:30 occurrence must be suppressed)", fire2, wantFire2)
}
}
// TestNextCronTime_DSTFallBack_DenseExpression_NoDoubleFire is a regression
// test for a fall-back suppression that only re-checked the single
// replacement candidate: an expression with multiple matching slots inside
// the repeated hour must not double-fire on any of them.
func TestNextCronTime_DSTFallBack_DenseExpression_NoDoubleFire(t *testing.T) {
loc, err := time.LoadLocation("America/New_York")
if err != nil {
t.Fatalf("load location: %v", err)
}
// 2026-11-01 is fall-back in America/New_York; 01:00 and 01:30 each
// occur twice (EDT, then EST).
after := time.Date(2026, 10, 31, 12, 0, 0, 0, loc)
fire1, err := NextCronTime("0,30 1 * * *", "America/New_York", after)
if err != nil {
t.Fatalf("fire1: unexpected error: %v", err)
}
wantFire1 := time.Date(2026, 11, 1, 1, 0, 0, 0, loc)
if !fire1.Equal(wantFire1) {
t.Fatalf("fire1: got %v, want %v", fire1, wantFire1)
}
fire2, err := NextCronTime("0,30 1 * * *", "America/New_York", fire1)
if err != nil {
t.Fatalf("fire2: unexpected error: %v", err)
}
wantFire2 := time.Date(2026, 11, 1, 1, 30, 0, 0, loc)
if !fire2.Equal(wantFire2) {
t.Fatalf("fire2: got %v, want %v", fire2, wantFire2)
}
// Both repeated fall-back occurrences (EST 01:00 and 01:30) must be
// suppressed: the next fire is the following day's 01:00, not a
// same-day repeat of either slot.
fire3, err := NextCronTime("0,30 1 * * *", "America/New_York", fire2)
if err != nil {
t.Fatalf("fire3: unexpected error: %v", err)
}
wantFire3 := time.Date(2026, 11, 2, 1, 0, 0, 0, loc)
if !fire3.Equal(wantFire3) {
t.Errorf("fire3: got %v, want %v (both repeated fall-back slots must be suppressed)", fire3, wantFire3)
}
}
// TestNextCronTime_DSTFallBack_ZeroOffsetZone is a regression test for
// disambiguation logic that assumed time.Date resolves an ambiguous wall
// clock to its pre-transition offset. That assumption does not hold in
// zones whose standard-time UTC offset is zero (e.g. Europe/London), where
// it resolves to the later, repeated occurrence instead.
func TestNextCronTime_DSTFallBack_ZeroOffsetZone(t *testing.T) {
loc, err := time.LoadLocation("Europe/London")
if err != nil {
t.Fatalf("load location: %v", err)
}
// 2026-10-25 is fall-back in Europe/London; 01:30 occurs twice (BST
// then GMT). Build expectations from explicit UTC instants so the
// assertions don't themselves depend on ambiguous local reconstruction.
after := time.Date(2026, 10, 24, 12, 0, 0, 0, loc)
fire1, err := NextCronTime("30 1 * * *", "Europe/London", after)
if err != nil {
t.Fatalf("fire1: unexpected error: %v", err)
}
// First occurrence: 01:30 BST (UTC+1) = 00:30 UTC.
wantFire1 := time.Date(2026, 10, 25, 0, 30, 0, 0, time.UTC)
if !fire1.Equal(wantFire1) {
t.Fatalf("fire1: got %v (%s), want %v", fire1, fire1.In(loc), wantFire1)
}
fire2, err := NextCronTime("30 1 * * *", "Europe/London", fire1)
if err != nil {
t.Fatalf("fire2: unexpected error: %v", err)
}
// The second occurrence (01:30 GMT = 01:30 UTC) must be suppressed:
// the next fire is the following day, not the same-day repeat.
wantFire2 := time.Date(2026, 10, 26, 1, 30, 0, 0, time.UTC)
if !fire2.Equal(wantFire2) {
t.Errorf("fire2: got %v (%s), want %v", fire2, fire2.In(loc), wantFire2)
}
}
// TestNextCronTime_LordHoweSpringForward_MatchesWallClock is a regression
// test for a wall-clock mismatch unique to Australia/Lord_Howe, the only IANA
// zone with a 30-minute DST shift (+10:30 <-> +11:00 spring-forward). robfig's
// minute loop advances in absolute time: searching forward from the first
// occurrence of "30 1 * * *" (2026-10-04 01:30, still before the gap) for the
// *next* occurrence crosses the gap and can land on a minute that satisfies
// the schedule's minute mask under an hour the schedule never asked for
// (local 02:30 on the same spring-forward day, instead of the following
// day's 01:30).
func TestNextCronTime_LordHoweSpringForward_MatchesWallClock(t *testing.T) {
loc, err := time.LoadLocation("Australia/Lord_Howe")
if err != nil {
t.Fatalf("load location: %v", err)
}
// 2026-10-04 is spring-forward in Australia/Lord_Howe: 02:00 -> 02:30.
after := time.Date(2026, 10, 3, 12, 0, 0, 0, loc)
cases := []struct {
name string
expr string
wantFire1Min int
wantFire2Day int
wantFire2Min int
}{
{"single slot", "30 1 * * *", 30, 5, 30},
{"first of two slots", "0,30 1 * * *", 0, 4, 30},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
fire1, err := NextCronTime(tc.expr, "Australia/Lord_Howe", after)
if err != nil {
t.Fatalf("fire1: unexpected error: %v", err)
}
local1 := fire1.In(loc)
if local1.Day() != 4 || local1.Hour() != 1 || local1.Minute() != tc.wantFire1Min {
t.Fatalf("fire1: expr %q: got local %v, want 2026-10-04 01:%02d", tc.expr, local1, tc.wantFire1Min)
}
fire2, err := NextCronTime(tc.expr, "Australia/Lord_Howe", fire1)
if err != nil {
t.Fatalf("fire2: unexpected error: %v", err)
}
local2 := fire2.In(loc)
if local2.Day() != tc.wantFire2Day || local2.Hour() != 1 || local2.Minute() != tc.wantFire2Min {
t.Errorf("fire2: expr %q: got local %v (hour=%d), want 2026-10-%02d 01:%02d",
tc.expr, local2, local2.Hour(), tc.wantFire2Day, tc.wantFire2Min)
}
})
}
}
// TestNextCronTime_LordHoweSweep_WallClockMatchesExpression sweeps every fire
// of several hour/minute expressions across calendar 2026 in
// Australia/Lord_Howe and asserts each fire's local wall clock actually
// satisfies the expression. Fire completeness (every genuinely-existing slot
// is returned) is covered separately by
// TestNextCronTime_LordHoweDailyExpression_FiresEveryDay.
func TestNextCronTime_LordHoweSweep_WallClockMatchesExpression(t *testing.T) {
loc, err := time.LoadLocation("Australia/Lord_Howe")
if err != nil {
t.Fatalf("load location: %v", err)
}
cases := []struct {
expr string
wantHour int
wantMinutes map[int]bool
}{
{"0 0 * * *", 0, map[int]bool{0: true}},
{"0 2 * * *", 2, map[int]bool{0: true}},
{"30 1 * * *", 1, map[int]bool{30: true}},
{"0,30 1 * * *", 1, map[int]bool{0: true, 30: true}},
}
yearEnd := time.Date(2027, 1, 1, 0, 0, 0, 0, loc)
for _, tc := range cases {
t.Run(tc.expr, func(t *testing.T) {
cursor := time.Date(2026, 1, 1, 0, 0, 0, 0, loc)
count := 0
for {
next, err := NextCronTime(tc.expr, "Australia/Lord_Howe", cursor)
if err != nil {
t.Fatalf("unexpected error at %v: %v", cursor, err)
}
if !next.Before(yearEnd) {
break
}
local := next.In(loc)
if local.Hour() != tc.wantHour || !tc.wantMinutes[local.Minute()] {
t.Errorf("expr %q: fire %v has hour=%d minute=%d, not one of the expression's slots",
tc.expr, local, local.Hour(), local.Minute())
}
cursor = next
count++
if count > 800 {
t.Fatalf("too many fires for a daily/twice-daily expression over one year; possible infinite loop")
}
}
if count == 0 {
t.Fatalf("expected at least one fire for %q", tc.expr)
}
})
}
}
// TestNextCronTime_LordHoweDailyExpression_FiresEveryDay is the regression
// test for the sub-hour-transition rescan: robfig/cron/v3's day-loop DST
// correction nudges by whole hours, so in Australia/Lord_Howe (the only IANA
// zone with a 30-minute DST shift) it can skip every fire scheduled anywhere
// on the transition day, not merely fires inside the 30-minute transition
// window. NextCronTime recovers those fires with a minute-granularity rescan
// gated on the sub-hour transition, so "0 12 * * *" now fires on every day of
// 2026, including both transition days, matching the
// TestNextCronTime_ChathamDailyExpression_FiresEveryDay control below.
func TestNextCronTime_LordHoweDailyExpression_FiresEveryDay(t *testing.T) {
loc, err := time.LoadLocation("Australia/Lord_Howe")
if err != nil {
t.Fatalf("load location: %v", err)
}
fallBack, err := NextCronTime("0 12 * * *", "Australia/Lord_Howe", time.Date(2026, 10, 3, 12, 0, 0, 0, loc))
if err != nil {
t.Fatalf("fall-back: unexpected error: %v", err)
}
if got := fallBack.In(loc); got.Year() != 2026 || got.Month() != time.October || got.Day() != 4 || got.Hour() != 12 || got.Minute() != 0 {
t.Errorf("fall-back: got %v, want 2026-10-04 12:00", got)
}
springForward, err := NextCronTime("0 12 * * *", "Australia/Lord_Howe", time.Date(2026, 4, 4, 12, 0, 0, 0, loc))
if err != nil {
t.Fatalf("spring-forward: unexpected error: %v", err)
}
if got := springForward.In(loc); got.Year() != 2026 || got.Month() != time.April || got.Day() != 5 || got.Hour() != 12 || got.Minute() != 0 {
t.Errorf("spring-forward: got %v, want 2026-04-05 12:00", got)
}
cursor := time.Date(2026, 1, 1, 0, 0, 0, 0, loc)
yearEnd := time.Date(2027, 1, 1, 0, 0, 0, 0, loc)
days := 0
for {
next, err := NextCronTime("0 12 * * *", "Australia/Lord_Howe", cursor)
if err != nil {
t.Fatalf("unexpected error at %v: %v", cursor, err)
}
if !next.Before(yearEnd) {
break
}
days++
cursor = next
}
if days != 365 {
t.Errorf("expected a fire on every one of 2026's 365 days, got %d", days)
}
}
// TestNextCronTime_ChathamDailyExpression_FiresEveryDay is the control for
// TestNextCronTime_LordHoweDailyExpression_FiresEveryDay: Pacific/Chatham
// has a whole-hour DST shift, so it never needed the sub-hour-transition
// rescan — "0 12 * * *" fires on every day of 2026, including both
// transition days.
func TestNextCronTime_ChathamDailyExpression_FiresEveryDay(t *testing.T) {
loc, err := time.LoadLocation("Pacific/Chatham")
if err != nil {
t.Fatalf("load location: %v", err)
}
cursor := time.Date(2026, 1, 1, 0, 0, 0, 0, loc)
yearEnd := time.Date(2027, 1, 1, 0, 0, 0, 0, loc)
days := 0
for {
next, err := NextCronTime("0 12 * * *", "Pacific/Chatham", cursor)
if err != nil {
t.Fatalf("unexpected error at %v: %v", cursor, err)
}
if !next.Before(yearEnd) {
break
}
days++
cursor = next
}
if days != 365 {
t.Errorf("expected a fire on every one of 2026's 365 days, got %d", days)
}
}
// TestNextCronTime_Unsatisfiable verifies an impossible expression (Feb 30th)
// returns ErrUnsatisfiableCron instead of a silent +24h fallback.
func TestNextCronTime_Unsatisfiable(t *testing.T) {
_, err := NextCronTime("0 0 30 2 *", "", time.Date(2026, 4, 25, 10, 0, 0, 0, time.UTC))
if err == nil {
t.Fatal("expected error for unsatisfiable expression")
}
if !errors.Is(err, ErrUnsatisfiableCron) {
t.Errorf("expected ErrUnsatisfiableCron, got %v", err)
}
}
func TestNextCronTime_InvalidTimezone(t *testing.T) {
_, err := NextCronTime("* * * * *", "Not/AZone", time.Now())
if err == nil {
t.Fatal("expected error for invalid timezone")
}
}
// TestNextCronTime_RejectsCronTZPrefix verifies that a caller-supplied
// TZ=/CRON_TZ= prefix is rejected rather than silently accepted. robfig/cron
// strips the prefix in Parse() before any field-mask check, so without this
// guard the expression's own prefix would override the trigger's timezone
// column and bypass the DST fall-back suppression (the candidate would carry
// the column's location while the schedule actually ran in the prefix's
// zone).
func TestNextCronTime_RejectsCronTZPrefix(t *testing.T) {
exprs := []string{
"CRON_TZ=America/New_York 30 1 * * *",
"TZ=Asia/Tokyo 0 9 * * *",
}
for _, expr := range exprs {
t.Run(expr, func(t *testing.T) {
_, err := NextCronTime(expr, "UTC", time.Now())
if err == nil {
t.Fatalf("expected error for prefixed expression %q, got none", expr)
}
})
}
}
@@ -39,6 +39,10 @@ queue](../../tasks/requirements/run-scheduling.md) and
- **AC-OFFICE-SCHEDULER-001.6:** Has a `source` discriminator (see table below) plus a typed payload.
- **AC-OFFICE-SCHEDULER-001.7:** Carries an `idempotency_key`. The queue uses a 24-hour lookup for recent duplicates and a durable unique key for persisted identities.
- **AC-OFFICE-SCHEDULER-001.8:** Is coalesced into an in-flight run when one exists for the same agent (claim-time merge).
- **AC-OFFICE-SCHEDULER-001.9:** A cron routine trigger's day-of-month and day-of-week fields are ORed when both are restricted, matching `crontab(5)` (`0 0 13 * 5` fires on the 13th of the month OR any Friday, not only Friday the 13th).
- **AC-OFFICE-SCHEDULER-001.10:** A cron routine trigger's wall-clock slot fires at most once across a DST transition: a slot that does not exist (spring-forward) is skipped, a slot that occurs twice (fall-back) fires only on its first occurrence, and no existing slot is ever lost. This holds for every IANA zone, including `Australia/Lord_Howe`, the only zone with a 30-minute DST shift.
- **AC-OFFICE-SCHEDULER-001.11:** A cron routine trigger that can never fire (an impossible date, or an empty expression) is rejected at trigger-create time with a client error, not accepted as a silent no-op or a wrong daily fallback.
- **AC-OFFICE-SCHEDULER-001.12:** A routine trigger's timezone defaults to UTC when not supplied; there is no workspace-level timezone.
## System design
@@ -118,7 +118,7 @@ Routine fields:
- `variables`: declared template variables (type, default, required).
Triggers:
- **Schedule (cron)**: `cron_expression`, `timezone`, computed `next_run_at`, `last_fired_at`.
- **Schedule (cron)**: `cron_expression`, `timezone`, computed `next_run_at`, `last_fired_at`. `cron_expression` is the standard 5-field syntax (minute hour day-of-month month day-of-week), computed via `robfig/cron/v3`. Day-of-month and day-of-week are ORed when both are restricted, matching `crontab(5)` (`0 0 13 * 5` fires on the 13th of the month OR any Friday). A wall-clock slot fires at most once across a DST transition: a spring-forward slot that does not exist is skipped, and a fall-back slot that occurs twice fires only on its first occurrence. Every returned fire is validated against the expression's own wall-clock fields (minute/hour/month/day-of-month-or-day-of-week), guarding against the underlying library returning a fire under the wrong hour when a DST shift crosses a match boundary. In `Australia/Lord_Howe` (the only IANA zone with a 30-minute DST shift, +10:30 <-> +11:00), `robfig/cron/v3`'s day-loop DST correction — which nudges by whole hours — can skip a candidate that genuinely exists; a minute-granularity rescan of the gap, gated on the interval containing a non-whole-hour offset transition, recovers it, so no zone loses an existing slot. `timezone` defaults to UTC when empty. An expression that can never fire (an impossible date, or empty) is rejected at trigger-create time.
- **Webhook**: `public_id` (URL path component), `signing_mode` (`none` | `bearer` | `hmac_sha256`), `secret`. URL: `POST /api/routine-triggers/<public_id>/fire`. Webhook payload is available as variables.
- **Manual**: fired only via UI or API.
@@ -165,7 +165,8 @@ variables: []
trigger:
kind: schedule
cron_expression: "*/5 * * * *"
timezone: (workspace TZ, fall back to UTC)
timezone: UTC (no workspace-level timezone exists; a trigger's timezone
defaults to UTC unless the user sets one explicitly)
enabled: true
```
@@ -86,7 +86,7 @@ id TEXT PRIMARY KEY
routine_id TEXT NOT NULL
kind TEXT NOT NULL -- schedule | webhook | manual
cron_expression TEXT -- when kind=schedule
timezone TEXT
timezone TEXT -- default 'UTC'; empty on legacy rows is backfilled to 'UTC'
next_run_at TIMESTAMP -- computed; atomically claimed for cron scheduling
last_fired_at TIMESTAMP
public_id TEXT UNIQUE -- when kind=webhook