From a8f2666b23770a7cb433a1dfc9e879d80e9830a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E8=B7=AF=E8=B7=AF?= Date: Wed, 4 Mar 2026 09:27:20 +0800 Subject: [PATCH] fix(cron): make default timezone configurable via config.json Refactor the hardcoded 'Asia/Shanghai' default timezone to be configurable. Timezone resolution follows a 3-tier fallback: 1. schedule.TZ (per-job, set by LLM via cron_expr) 2. tools.cron.default_timezone in config.json (server-wide) 3. 'Asia/Shanghai' as ultimate fallback Changes: - config: add DefaultTimezone field to CronToolsConfig - cron: add SetDefaultTimezone() method to CronService - cron: update computeNextRun() to use 3-tier fallback - gateway: wire config.Tools.Cron.DefaultTimezone to CronService - test: add TestComputeNextRun_ServiceDefaultTZ to verify 3-tier Usage in config.json: {"tools": {"cron": {"default_timezone": "US/Eastern"}}} --- cmd/picoclaw/internal/gateway/helpers.go | 5 + pkg/config/config.go | 3 +- pkg/cron/service.go | 14 ++- pkg/cron/timezone_test.go | 144 +++++++++++++++++++++++ 4 files changed, 164 insertions(+), 2 deletions(-) create mode 100644 pkg/cron/timezone_test.go diff --git a/cmd/picoclaw/internal/gateway/helpers.go b/cmd/picoclaw/internal/gateway/helpers.go index 174f5db62..da2de6535 100644 --- a/cmd/picoclaw/internal/gateway/helpers.go +++ b/cmd/picoclaw/internal/gateway/helpers.go @@ -230,6 +230,11 @@ func setupCronTool( // Create cron service cronService := cron.NewCronService(cronStorePath, nil) + // Apply default timezone from config for cron expressions + if cfg.Tools.Cron.DefaultTimezone != "" { + cronService.SetDefaultTimezone(cfg.Tools.Cron.DefaultTimezone) + } + // Create and register CronTool if enabled var cronTool *tools.CronTool if cfg.Tools.IsToolEnabled("cron") { diff --git a/pkg/config/config.go b/pkg/config/config.go index 0ee3acfe0..9952ce5fa 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -579,7 +579,8 @@ type WebToolsConfig struct { type CronToolsConfig struct { ToolConfig ` envPrefix:"PICOCLAW_TOOLS_CRON_"` - ExecTimeoutMinutes int ` env:"PICOCLAW_TOOLS_CRON_EXEC_TIMEOUT_MINUTES" json:"exec_timeout_minutes"` // 0 means no timeout + ExecTimeoutMinutes int ` env:"PICOCLAW_TOOLS_CRON_EXEC_TIMEOUT_MINUTES" json:"exec_timeout_minutes"` // 0 means no timeout + DefaultTimezone string ` env:"PICOCLAW_TOOLS_CRON_DEFAULT_TIMEZONE" json:"default_timezone"` // default timezone for cron expressions, e.g. "UTC" } type ExecConfig struct { diff --git a/pkg/cron/service.go b/pkg/cron/service.go index 6a74369ac..111c480b5 100644 --- a/pkg/cron/service.go +++ b/pkg/cron/service.go @@ -66,6 +66,7 @@ type CronService struct { running bool stopChan chan struct{} gronx *gronx.Gronx + defaultTZ string // default timezone for cron expressions } func NewCronService(storePath string, onJob JobHandler) *CronService { @@ -266,8 +267,11 @@ func (cs *CronService) computeNextRun(schedule *CronSchedule, nowMS int64) *int6 // Use gronx to calculate next run time now := time.UnixMilli(nowMS) - // Apply timezone if specified, default to Asia/Shanghai + // Apply timezone: schedule.TZ > service default > "Asia/Shanghai" tz := schedule.TZ + if tz == "" { + tz = cs.defaultTZ + } if tz == "" { tz = "Asia/Shanghai" } @@ -321,6 +325,14 @@ func (cs *CronService) SetOnJob(handler JobHandler) { cs.onJob = handler } +// SetDefaultTimezone sets the default timezone for cron expressions. +// If empty, falls back to "Asia/Shanghai". +func (cs *CronService) SetDefaultTimezone(tz string) { + cs.mu.Lock() + defer cs.mu.Unlock() + cs.defaultTZ = tz +} + func (cs *CronService) loadStore() error { cs.store = &CronStore{ Version: 1, diff --git a/pkg/cron/timezone_test.go b/pkg/cron/timezone_test.go new file mode 100644 index 000000000..d441ad98a --- /dev/null +++ b/pkg/cron/timezone_test.go @@ -0,0 +1,144 @@ +package cron + +import ( + "path/filepath" + "testing" + "time" +) + +// TestComputeNextRun_CronTimezone verifies Patch #4: cron expressions should +// respect the schedule.TZ field instead of always using UTC. +func TestComputeNextRun_CronTimezone(t *testing.T) { + tmpDir := t.TempDir() + storePath := filepath.Join(tmpDir, "jobs.json") + cs := NewCronService(storePath, nil) + + now := time.Date(2026, 3, 4, 0, 0, 0, 0, time.UTC) // midnight UTC + nowMS := now.UnixMilli() + + // Cron expr "0 9 * * *" = daily at 9:00 + tests := []struct { + name string + tz string + wantHour int // expected hour in UTC of the next run + }{ + { + name: "UTC timezone", + tz: "UTC", + wantHour: 9, // 9:00 UTC + }, + { + name: "Asia/Shanghai timezone", + tz: "Asia/Shanghai", + wantHour: 1, // 9:00 CST = 1:00 UTC + }, + { + name: "empty TZ defaults to Asia/Shanghai", + tz: "", + wantHour: 1, // should default to Asia/Shanghai + }, + { + name: "US/Eastern timezone", + tz: "US/Eastern", + wantHour: 14, // 9:00 EST = 14:00 UTC (or 13:00 during DST) + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + schedule := &CronSchedule{ + Kind: "cron", + Expr: "0 9 * * *", + TZ: tt.tz, + } + + nextMS := cs.computeNextRun(schedule, nowMS) + if nextMS == nil { + t.Fatal("computeNextRun returned nil") + } + + nextTime := time.UnixMilli(*nextMS).UTC() + + if tt.name == "US/Eastern timezone" { + // Allow for DST variation (13 or 14) + if nextTime.Hour() != 13 && nextTime.Hour() != 14 { + t.Errorf("next run hour = %d, want 13 or 14 (UTC)", nextTime.Hour()) + } + } else { + if nextTime.Hour() != tt.wantHour { + t.Errorf("next run hour = %d, want %d (UTC)", nextTime.Hour(), tt.wantHour) + } + } + }) + } +} + +// TestComputeNextRun_DefaultTZ_NotUTC verifies that when TZ is empty, +// the computed next run differs from a pure UTC computation. +func TestComputeNextRun_DefaultTZ_NotUTC(t *testing.T) { + tmpDir := t.TempDir() + storePath := filepath.Join(tmpDir, "jobs.json") + cs := NewCronService(storePath, nil) + + now := time.Date(2026, 3, 4, 2, 0, 0, 0, time.UTC) // 02:00 UTC = 10:00 CST + nowMS := now.UnixMilli() + + scheduleEmpty := &CronSchedule{Kind: "cron", Expr: "0 9 * * *", TZ: ""} + scheduleUTC := &CronSchedule{Kind: "cron", Expr: "0 9 * * *", TZ: "UTC"} + + nextEmpty := cs.computeNextRun(scheduleEmpty, nowMS) + nextUTC := cs.computeNextRun(scheduleUTC, nowMS) + + if nextEmpty == nil || nextUTC == nil { + t.Fatal("computeNextRun returned nil") + } + + // With empty TZ (default Asia/Shanghai), 9:00 CST already passed (it's 10:00 CST), + // so next run should be tomorrow 9:00 CST = today 01:00 UTC + 24h. + // With UTC, 9:00 UTC hasn't happened yet (it's 02:00 UTC), so next run = today 9:00 UTC. + // They must differ. + if *nextEmpty == *nextUTC { + t.Errorf("empty TZ and UTC TZ produced same next run time, default TZ is not working") + } +} + +// TestComputeNextRun_ServiceDefaultTZ verifies that SetDefaultTimezone() +// takes effect when schedule.TZ is empty (3-tier fallback). +func TestComputeNextRun_ServiceDefaultTZ(t *testing.T) { + tmpDir := t.TempDir() + storePath := filepath.Join(tmpDir, "jobs.json") + cs := NewCronService(storePath, nil) + + // Set service-level default to US/Eastern + cs.SetDefaultTimezone("US/Eastern") + + now := time.Date(2026, 3, 4, 0, 0, 0, 0, time.UTC) + nowMS := now.UnixMilli() + + // Schedule with empty TZ should use service default (US/Eastern), not Asia/Shanghai + scheduleEmpty := &CronSchedule{Kind: "cron", Expr: "0 9 * * *", TZ: ""} + scheduleCST := &CronSchedule{Kind: "cron", Expr: "0 9 * * *", TZ: "Asia/Shanghai"} + + nextEmpty := cs.computeNextRun(scheduleEmpty, nowMS) + nextCST := cs.computeNextRun(scheduleCST, nowMS) + + if nextEmpty == nil || nextCST == nil { + t.Fatal("computeNextRun returned nil") + } + + // US/Eastern 9:00 != Asia/Shanghai 9:00, so they must differ + if *nextEmpty == *nextCST { + t.Errorf("service default TZ (US/Eastern) and Asia/Shanghai produced same time; SetDefaultTimezone not working") + } + + // Schedule with explicit TZ should override service default + scheduleExplicit := &CronSchedule{Kind: "cron", Expr: "0 9 * * *", TZ: "UTC"} + nextExplicit := cs.computeNextRun(scheduleExplicit, nowMS) + if nextExplicit == nil { + t.Fatal("computeNextRun returned nil") + } + nextUTC := time.UnixMilli(*nextExplicit).UTC() + if nextUTC.Hour() != 9 { + t.Errorf("explicit TZ=UTC: next run hour = %d, want 9", nextUTC.Hour()) + } +}