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"}}}
This commit is contained in:
parent
150bb34e51
commit
a8f2666b23
4 changed files with 164 additions and 2 deletions
|
|
@ -230,6 +230,11 @@ func setupCronTool(
|
||||||
// Create cron service
|
// Create cron service
|
||||||
cronService := cron.NewCronService(cronStorePath, nil)
|
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
|
// Create and register CronTool if enabled
|
||||||
var cronTool *tools.CronTool
|
var cronTool *tools.CronTool
|
||||||
if cfg.Tools.IsToolEnabled("cron") {
|
if cfg.Tools.IsToolEnabled("cron") {
|
||||||
|
|
|
||||||
|
|
@ -579,7 +579,8 @@ type WebToolsConfig struct {
|
||||||
|
|
||||||
type CronToolsConfig struct {
|
type CronToolsConfig struct {
|
||||||
ToolConfig ` envPrefix:"PICOCLAW_TOOLS_CRON_"`
|
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 {
|
type ExecConfig struct {
|
||||||
|
|
|
||||||
|
|
@ -66,6 +66,7 @@ type CronService struct {
|
||||||
running bool
|
running bool
|
||||||
stopChan chan struct{}
|
stopChan chan struct{}
|
||||||
gronx *gronx.Gronx
|
gronx *gronx.Gronx
|
||||||
|
defaultTZ string // default timezone for cron expressions
|
||||||
}
|
}
|
||||||
|
|
||||||
func NewCronService(storePath string, onJob JobHandler) *CronService {
|
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
|
// Use gronx to calculate next run time
|
||||||
now := time.UnixMilli(nowMS)
|
now := time.UnixMilli(nowMS)
|
||||||
// Apply timezone if specified, default to Asia/Shanghai
|
// Apply timezone: schedule.TZ > service default > "Asia/Shanghai"
|
||||||
tz := schedule.TZ
|
tz := schedule.TZ
|
||||||
|
if tz == "" {
|
||||||
|
tz = cs.defaultTZ
|
||||||
|
}
|
||||||
if tz == "" {
|
if tz == "" {
|
||||||
tz = "Asia/Shanghai"
|
tz = "Asia/Shanghai"
|
||||||
}
|
}
|
||||||
|
|
@ -321,6 +325,14 @@ func (cs *CronService) SetOnJob(handler JobHandler) {
|
||||||
cs.onJob = handler
|
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 {
|
func (cs *CronService) loadStore() error {
|
||||||
cs.store = &CronStore{
|
cs.store = &CronStore{
|
||||||
Version: 1,
|
Version: 1,
|
||||||
|
|
|
||||||
144
pkg/cron/timezone_test.go
Normal file
144
pkg/cron/timezone_test.go
Normal file
|
|
@ -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())
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Add table
Reference in a new issue