From 6324427862bd898d3805c4eb3ae6c7375aca50ac Mon Sep 17 00:00:00 2001 From: liugangjian Date: Mon, 9 Mar 2026 09:27:56 +0800 Subject: [PATCH] Fix #1044: Cron expressions respect schedule.TZ field using 3-level fallback Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus --- cmd/picoclaw/internal/gateway/helpers.go | 10 ++++- config/config.example.json | 3 +- pkg/config/config.go | 3 +- pkg/cron/service.go | 49 +++++++++++++++++------- pkg/tools/cron.go | 5 +++ 5 files changed, 53 insertions(+), 17 deletions(-) diff --git a/cmd/picoclaw/internal/gateway/helpers.go b/cmd/picoclaw/internal/gateway/helpers.go index 747f7d44e..062f7762b 100644 --- a/cmd/picoclaw/internal/gateway/helpers.go +++ b/cmd/picoclaw/internal/gateway/helpers.go @@ -221,8 +221,14 @@ func setupCronTool( cronStorePath := filepath.Join(workspace, "cron", "jobs.json") // Create cron service - cronService := cron.NewCronService(cronStorePath, nil) - + cronService := cron.NewCronService( + cronStorePath, + nil, + cron.CronConfig{ + ExecTimeoutMinutes: cfg.Tools.Cron.ExecTimeoutMinutes, + DefaultTimezone: cfg.Tools.Cron.DefaultTimezone, + }, + ) // Create and register CronTool cronTool, err := tools.NewCronTool(cronService, agentLoop, msgBus, workspace, restrict, execTimeout, cfg) if err != nil { diff --git a/config/config.example.json b/config/config.example.json index adae6f05c..5b0e50b36 100644 --- a/config/config.example.json +++ b/config/config.example.json @@ -243,7 +243,8 @@ "proxy": "" }, "cron": { - "exec_timeout_minutes": 5 + "exec_timeout_minutes": 5, + "default_timezone": "Asia/Shanghai" }, "mcp": { "enabled": false, diff --git a/pkg/config/config.go b/pkg/config/config.go index f40e05e1c..a6a4e0afb 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -570,7 +570,8 @@ type WebToolsConfig struct { } type CronToolsConfig struct { - ExecTimeoutMinutes int `json:"exec_timeout_minutes" env:"PICOCLAW_TOOLS_CRON_EXEC_TIMEOUT_MINUTES"` // 0 means no timeout + ExecTimeoutMinutes int `json:"exec_timeout_minutes" env:"PICOCLAW_TOOLS_CRON_EXEC_TIMEOUT_MINUTES"` // 0 means no timeout + DefaultTimezone string `json:"default_timezone" env:"PICOCLAW_TOOLS_CRON_DEFAULT_TIMEZONE"` } type ExecConfig struct { diff --git a/pkg/cron/service.go b/pkg/cron/service.go index 6962041c1..d2bde0596 100644 --- a/pkg/cron/service.go +++ b/pkg/cron/service.go @@ -14,6 +14,10 @@ import ( "github.com/sipeed/picoclaw/pkg/fileutil" ) +type CronConfig struct { + ExecTimeoutMinutes int `json:"exec_timeout_minutes,omitempty"` + DefaultTimezone string `json:"default_timezone,omitempty"` +} type CronSchedule struct { Kind string `json:"kind"` @@ -59,20 +63,22 @@ type CronStore struct { type JobHandler func(job *CronJob) (string, error) type CronService struct { - storePath string - store *CronStore - onJob JobHandler - mu sync.RWMutex - running bool - stopChan chan struct{} +storePath string +store *CronStore +onJob JobHandler +mu sync.RWMutex +running bool +stopChan chan struct{} gronx *gronx.Gronx + config CronConfig } -func NewCronService(storePath string, onJob JobHandler) *CronService { +func NewCronService(storePath string, onJob JobHandler, config CronConfig) *CronService { cs := &CronService{ storePath: storePath, onJob: onJob, gronx: gronx.New(), + config: config, } // Initialize and load store on creation cs.loadStore() @@ -263,16 +269,33 @@ func (cs *CronService) computeNextRun(schedule *CronSchedule, nowMS int64) *int6 if schedule.Expr == "" { return nil } - - // Use gronx to calculate next run time - now := time.UnixMilli(nowMS) + + // 3-level fallback: schedule.TZ > config default_timezone > "Asia/Shanghai" + timezoneStr := schedule.TZ + if timezoneStr == "" { + timezoneStr = cs.config.DefaultTimezone + } + if timezoneStr == "" { + timezoneStr = "Asia/Shanghai" // Default fallback + } + + // Load the target timezone + targetTZ, err := time.LoadLocation(timezoneStr) + if err != nil { + log.Printf("[cron] failed to load timezone '%s', falling back to UTC: %v", timezoneStr, err) + targetTZ = time.UTC // fallback to UTC on error + } + + // Use gronx to calculate next run time based on target timezone + now := time.UnixMilli(nowMS).In(targetTZ) nextTime, err := gronx.NextTickAfter(schedule.Expr, now, false) if err != nil { - log.Printf("[cron] failed to compute next run for expr '%s': %v", schedule.Expr, err) + log.Printf("[cron] failed to compute next run for expr '%s' in timezone '%s': %v", schedule.Expr, timezoneStr, err) return nil } - - nextMS := nextTime.UnixMilli() + + // Convert the calculated next time back to UTC Unix milli for storage + nextMS := nextTime.UTC().UnixMilli() return &nextMS } diff --git a/pkg/tools/cron.go b/pkg/tools/cron.go index 6888d1326..f28bdedf1 100644 --- a/pkg/tools/cron.go +++ b/pkg/tools/cron.go @@ -173,6 +173,11 @@ func (t *CronTool) addJob(args map[string]any) *ToolResult { Kind: "cron", Expr: cronExpr, } + + // Get timezone if present in args + if tz, ok := args["timezone"].(string); ok && tz != "" { + schedule.TZ = tz // Set timezone on cron schedule + } } else { return ErrorResult("one of at_seconds, every_seconds, or cron_expr is required") }