Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module github.com/sipeed/picoclaw

go 1.25.7
go 1.23

require (
github.com/adhocore/gronx v1.19.6
Expand Down
4 changes: 3 additions & 1 deletion pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,9 @@ 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
AllowCommand bool ` env:"PICOCLAW_TOOLS_CRON_ALLOW_COMMAND" json:"allow_command"`
MinIntervalSeconds int ` env:"PICOCLAW_TOOLS_CRON_MIN_INTERVAL_SECONDS" json:"min_interval_seconds"` // 0 means disable check, default 60
}

type ExecConfig struct {
Expand Down
2 changes: 2 additions & 0 deletions pkg/config/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,8 @@ func DefaultConfig() *Config {
Enabled: true,
},
ExecTimeoutMinutes: 5,
AllowCommand: true,
MinIntervalSeconds: 60,
},
Exec: ExecConfig{
ToolConfig: ToolConfig{
Expand Down
75 changes: 65 additions & 10 deletions pkg/tools/cron.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"strings"
"time"

"github.com/adhocore/gronx"

"github.com/sipeed/picoclaw/pkg/bus"
"github.com/sipeed/picoclaw/pkg/config"
"github.com/sipeed/picoclaw/pkg/cron"
Expand All @@ -19,29 +21,40 @@ type JobExecutor interface {

// CronTool provides scheduling capabilities for the agent
type CronTool struct {
cronService *cron.CronService
executor JobExecutor
msgBus *bus.MessageBus
execTool *ExecTool
cronService *cron.CronService
executor JobExecutor
msgBus *bus.MessageBus
execTool *ExecTool
minIntervalSeconds int
}

// NewCronTool creates a new CronTool
// execTimeout: 0 means no timeout, >0 sets the timeout duration
func NewCronTool(
cronService *cron.CronService, executor JobExecutor, msgBus *bus.MessageBus, workspace string, restrict bool,
execTimeout time.Duration, config *config.Config,
execTimeout time.Duration, cfg *config.Config,
) (*CronTool, error) {
execTool, err := NewExecToolWithConfig(workspace, restrict, config)
execTool, err := NewExecToolWithConfig(workspace, restrict, cfg)
if err != nil {
return nil, fmt.Errorf("unable to configure exec tool: %w", err)
}

execTool.SetTimeout(execTimeout)

minIntervalSeconds := 60 // default 1 minute
if cfg != nil {
minIntervalSeconds = cfg.Tools.Cron.MinIntervalSeconds
if minIntervalSeconds < 0 {
minIntervalSeconds = 60
}
}

return &CronTool{
cronService: cronService,
executor: executor,
msgBus: msgBus,
execTool: execTool,
cronService: cronService,
executor: executor,
msgBus: msgBus,
execTool: execTool,
minIntervalSeconds: minIntervalSeconds,
}, nil
}

Expand Down Expand Up @@ -121,6 +134,41 @@ func (t *CronTool) Execute(ctx context.Context, args map[string]any) *ToolResult
}
}

// validateMinInterval checks if the schedule interval meets the minimum requirement
func (t *CronTool) validateMinInterval(schedule cron.CronSchedule) error {
if t.minIntervalSeconds <= 0 {
return nil
}

switch schedule.Kind {
case "every":
if schedule.EveryMS != nil {
intervalSeconds := *schedule.EveryMS / 1000
if intervalSeconds < int64(t.minIntervalSeconds) {
return fmt.Errorf("interval too short: minimum allowed is %d seconds, got %d seconds", t.minIntervalSeconds, intervalSeconds)
}
}
case "cron":
if schedule.Expr != "" {
// Calculate the interval between next two ticks
now := time.Now()
next1, err := gronx.NextTickAfter(schedule.Expr, now, false)
if err != nil {
return fmt.Errorf("invalid cron expression: %w", err)
}
next2, err := gronx.NextTickAfter(schedule.Expr, next1, false)
if err != nil {
return fmt.Errorf("invalid cron expression: %w", err)
}
intervalSeconds := int64(next2.Sub(next1).Seconds())
if intervalSeconds < int64(t.minIntervalSeconds) {
return fmt.Errorf("cron interval too short: minimum allowed is %d seconds, calculated interval is %d seconds", t.minIntervalSeconds, intervalSeconds)
}
}
}
return nil
}

func (t *CronTool) addJob(ctx context.Context, args map[string]any) *ToolResult {
channel := ToolChannel(ctx)
chatID := ToolChatID(ctx)
Expand Down Expand Up @@ -184,6 +232,13 @@ func (t *CronTool) addJob(ctx context.Context, args map[string]any) *ToolResult
deliver = false
}

// Validate minimum interval for recurring schedules
if t.minIntervalSeconds > 0 {
if err := t.validateMinInterval(schedule); err != nil {
return ErrorResult(err.Error())
}
}

// Truncate message for job name (max 30 chars)
messagePreview := utils.Truncate(message, 30)

Expand Down
185 changes: 185 additions & 0 deletions pkg/tools/cron_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
package tools

import (
"context"
"path/filepath"
"strings"
"testing"
"time"

"github.com/sipeed/picoclaw/pkg/bus"
"github.com/sipeed/picoclaw/pkg/config"
"github.com/sipeed/picoclaw/pkg/cron"
)

func newTestCronToolWithConfig(t *testing.T, cfg *config.Config) *CronTool {
t.Helper()
storePath := filepath.Join(t.TempDir(), "cron.json")
cronService := cron.NewCronService(storePath, nil)
msgBus := bus.NewMessageBus()
tool, err := NewCronTool(cronService, nil, msgBus, t.TempDir(), true, 0, cfg)
if err != nil {
t.Fatalf("NewCronTool() error: %v", err)
}
return tool
}

func newTestCronTool(t *testing.T) *CronTool {
t.Helper()
return newTestCronToolWithConfig(t, config.DefaultConfig())
}

// TestCronTool_MinIntervalSeconds validates that min_interval_seconds config prevents excessive scheduling
func TestCronTool_MinIntervalSeconds(t *testing.T) {
// Test with default config (min_interval_seconds = 60)
tool := newTestCronTool(t)
ctx := WithToolContext(context.Background(), "cli", "direct")

// Test every_seconds below minimum (should fail)
result := tool.Execute(ctx, map[string]any{
"action": "add",
"message": "test reminder",
"every_seconds": float64(30), // 30 seconds, below default 60
})

if !result.IsError {
t.Fatal("expected error for interval below minimum")
}
if !strings.Contains(result.ForLLM, "interval too short") {
t.Errorf("expected 'interval too short' error, got: %s", result.ForLLM)
}

// Test every_seconds at minimum (should succeed)
result = tool.Execute(ctx, map[string]any{
"action": "add",
"message": "test reminder",
"every_seconds": float64(60), // 60 seconds, at default minimum
})

if result.IsError {
t.Fatalf("expected success for interval at minimum, got: %s", result.ForLLM)
}
}

// TestCronTool_MinIntervalSecondsDisabled validates that min_interval_seconds = 0 disables the check
func TestCronTool_MinIntervalSecondsDisabled(t *testing.T) {
// Create config with min_interval_seconds = 0 (disabled)
cfg := config.DefaultConfig()
cfg.Tools.Cron.MinIntervalSeconds = 0

tool := newTestCronToolWithConfig(t, cfg)
ctx := WithToolContext(context.Background(), "cli", "direct")

// Test every_seconds below default minimum (should succeed when check is disabled)
result := tool.Execute(ctx, map[string]any{
"action": "add",
"message": "test reminder",
"every_seconds": float64(10), // 10 seconds, would fail if check was enabled
})

if result.IsError {
t.Fatalf("expected success when min_interval_seconds is disabled, got: %s", result.ForLLM)
}
}

// TestCronTool_MinIntervalSecondsCron validates cron expression interval checking
func TestCronTool_MinIntervalSecondsCron(t *testing.T) {
// Test with default config (min_interval_seconds = 60)
tool := newTestCronTool(t)
ctx := WithToolContext(context.Background(), "cli", "direct")

// Test cron expression with interval below minimum (every 30 seconds)
result := tool.Execute(ctx, map[string]any{
"action": "add",
"message": "test reminder",
"cron_expr": "*/30 * * * * *", // every 30 seconds
})

if !result.IsError {
t.Fatal("expected error for cron interval below minimum")
}
if !strings.Contains(result.ForLLM, "cron interval too short") {
t.Errorf("expected 'cron interval too short' error, got: %s", result.ForLLM)
}

// Test cron expression with interval at minimum (every minute)
result = tool.Execute(ctx, map[string]any{
"action": "add",
"message": "test reminder",
"cron_expr": "0 * * * * *", // every minute
})

if result.IsError {
t.Fatalf("expected success for cron interval at minimum, got: %s", result.ForLLM)
}
}

// TestCronTool_AtSecondsNotAffected validates that at_seconds is not affected by min_interval_seconds
func TestCronTool_AtSecondsNotAffected(t *testing.T) {
tool := newTestCronTool(t)
ctx := WithToolContext(context.Background(), "cli", "direct")

// Test at_seconds (one-time) should not be affected by min_interval_seconds
result := tool.Execute(ctx, map[string]any{
"action": "add",
"message": "test reminder",
"at_seconds": float64(10), // 10 seconds from now
})

if result.IsError {
t.Fatalf("expected success for at_seconds regardless of min_interval_seconds, got: %s", result.ForLLM)
}
}

// TestCronTool_MinIntervalSecondsCustom validates custom min_interval_seconds values
func TestCronTool_MinIntervalSecondsCustom(t *testing.T) {
// Create config with custom min_interval_seconds = 120
cfg := config.DefaultConfig()
cfg.Tools.Cron.MinIntervalSeconds = 120

tool := newTestCronToolWithConfig(t, cfg)
ctx := WithToolContext(context.Background(), "cli", "direct")

// Test every_seconds below custom minimum (should fail)
result := tool.Execute(ctx, map[string]any{
"action": "add",
"message": "test reminder",
"every_seconds": float64(90), // 90 seconds, below custom minimum 120
})

if !result.IsError {
t.Fatal("expected error for interval below custom minimum")
}

// Test every_seconds at custom minimum (should succeed)
result = tool.Execute(ctx, map[string]any{
"action": "add",
"message": "test reminder",
"every_seconds": float64(120), // 120 seconds, at custom minimum
})

if result.IsError {
t.Fatalf("expected success for interval at custom minimum, got: %s", result.ForLLM)
}
}

// TestCronTool_MinIntervalSecondsNegative validates that negative values are treated as default
func TestCronTool_MinIntervalSecondsNegative(t *testing.T) {
// Create config with negative min_interval_seconds (should use default 60)
cfg := config.DefaultConfig()
cfg.Tools.Cron.MinIntervalSeconds = -1

tool := newTestCronToolWithConfig(t, cfg)
ctx := WithToolContext(context.Background(), "cli", "direct")

// Test every_seconds below default minimum (should fail with default 60)
result := tool.Execute(ctx, map[string]any{
"action": "add",
"message": "test reminder",
"every_seconds": float64(30), // 30 seconds, below default 60
})

if !result.IsError {
t.Fatal("expected error for interval below default minimum when negative value provided")
}
}