feat(cron): add min_interval_seconds config
- Add MinIntervalSeconds field to CronToolsConfig (default: 60 seconds) - Add environment variable PICOCLAW_TOOLS_CRON_MIN_INTERVAL_SECONDS - Implement validateMinInterval() to check schedule intervals - For 'every' type: directly compare every_seconds with minimum - For 'cron' type: calculate next two ticks interval using gronx - Value 0 disables the check - Add comprehensive test cases Closes #1655
This commit is contained in:
parent
2312553286
commit
4d0a6e7ed6
5 changed files with 256 additions and 12 deletions
2
go.mod
2
go.mod
|
|
@ -1,6 +1,6 @@
|
|||
module github.com/sipeed/picoclaw
|
||||
|
||||
go 1.25.7
|
||||
go 1.23
|
||||
|
||||
require (
|
||||
github.com/adhocore/gronx v1.19.6
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -413,6 +413,8 @@ func DefaultConfig() *Config {
|
|||
Enabled: true,
|
||||
},
|
||||
ExecTimeoutMinutes: 5,
|
||||
AllowCommand: true,
|
||||
MinIntervalSeconds: 60,
|
||||
},
|
||||
Exec: ExecConfig{
|
||||
ToolConfig: ToolConfig{
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
@ -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
|
||||
}
|
||||
|
||||
|
|
@ -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)
|
||||
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
185
pkg/tools/cron_test.go
Normal file
185
pkg/tools/cron_test.go
Normal file
|
|
@ -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")
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue