feat(cron): add min_interval_seconds to prevent excessive scheduling
Add a configurable minimum interval for recurring cron jobs to prevent token waste and API abuse. The check applies to both 'every' and 'cron' schedule kinds; one-time 'at' jobs are unaffected. Default: 60 seconds. Set to 0 to disable. Fixes #1655 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
79b0568d75
commit
af12798960
6 changed files with 170 additions and 12 deletions
|
|
@ -104,9 +104,10 @@ By default, PicoClaw blocks the following dangerous commands:
|
|||
|
||||
The cron tool is used for scheduling periodic tasks.
|
||||
|
||||
| Config | Type | Default | Description |
|
||||
|------------------------|------|---------|------------------------------------------------|
|
||||
| `exec_timeout_minutes` | int | 5 | Execution timeout in minutes, 0 means no limit |
|
||||
| Config | Type | Default | Description |
|
||||
|------------------------|------|---------|--------------------------------------------------------------|
|
||||
| `exec_timeout_minutes` | int | 5 | Execution timeout in minutes, 0 means no limit |
|
||||
| `min_interval_seconds` | int | 60 | Minimum allowed interval for recurring jobs, 0 means no limit |
|
||||
|
||||
## MCP Tool
|
||||
|
||||
|
|
@ -311,6 +312,7 @@ For example:
|
|||
- `PICOCLAW_TOOLS_WEB_BRAVE_ENABLED=true`
|
||||
- `PICOCLAW_TOOLS_EXEC_ENABLE_DENY_PATTERNS=false`
|
||||
- `PICOCLAW_TOOLS_CRON_EXEC_TIMEOUT_MINUTES=10`
|
||||
- `PICOCLAW_TOOLS_CRON_MIN_INTERVAL_SECONDS=60`
|
||||
- `PICOCLAW_TOOLS_MCP_ENABLED=true`
|
||||
|
||||
Note: Nested map-style config (for example `tools.mcp.servers.<name>.*`) is configured in `config.json` rather than
|
||||
|
|
|
|||
|
|
@ -700,7 +700,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
|
||||
MinIntervalSeconds int ` env:"PICOCLAW_TOOLS_CRON_MIN_INTERVAL_SECONDS" json:"min_interval_seconds"` // minimum interval for recurring jobs; 0 means no limit
|
||||
}
|
||||
|
||||
type ExecConfig struct {
|
||||
|
|
|
|||
|
|
@ -452,6 +452,7 @@ func DefaultConfig() *Config {
|
|||
Enabled: true,
|
||||
},
|
||||
ExecTimeoutMinutes: 5,
|
||||
MinIntervalSeconds: 60,
|
||||
},
|
||||
Exec: ExecConfig{
|
||||
ToolConfig: ToolConfig{
|
||||
|
|
|
|||
|
|
@ -506,6 +506,11 @@ func (cs *CronService) Status() map[string]any {
|
|||
}
|
||||
}
|
||||
|
||||
// NextTickAfter returns the next time a cron expression fires after the given reference time.
|
||||
func NextTickAfter(expr string, after time.Time) (time.Time, error) {
|
||||
return gronx.NextTickAfter(expr, after, false)
|
||||
}
|
||||
|
||||
func generateID() string {
|
||||
// Use crypto/rand for better uniqueness under concurrent access
|
||||
b := make([]byte, 8)
|
||||
|
|
|
|||
|
|
@ -20,10 +20,11 @@ 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
|
||||
|
|
@ -39,10 +40,11 @@ func NewCronTool(
|
|||
|
||||
execTool.SetTimeout(execTimeout)
|
||||
return &CronTool{
|
||||
cronService: cronService,
|
||||
executor: executor,
|
||||
msgBus: msgBus,
|
||||
execTool: execTool,
|
||||
cronService: cronService,
|
||||
executor: executor,
|
||||
msgBus: msgBus,
|
||||
execTool: execTool,
|
||||
minIntervalSeconds: config.Tools.Cron.MinIntervalSeconds,
|
||||
}, nil
|
||||
}
|
||||
|
||||
|
|
@ -174,6 +176,13 @@ func (t *CronTool) addJob(ctx context.Context, args map[string]any) *ToolResult
|
|||
return ErrorResult("one of at_seconds, every_seconds, or cron_expr is required")
|
||||
}
|
||||
|
||||
// Enforce minimum interval for recurring schedules
|
||||
if t.minIntervalSeconds > 0 {
|
||||
if err := t.validateMinInterval(schedule); err != nil {
|
||||
return ErrorResult(err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
// Read deliver parameter, default to false so scheduled tasks execute through the agent
|
||||
deliver := false
|
||||
if d, ok := args["deliver"].(bool); ok {
|
||||
|
|
@ -274,6 +283,42 @@ func (t *CronTool) enableJob(args map[string]any, enable bool) *ToolResult {
|
|||
return SilentResult(fmt.Sprintf("Cron job '%s' %s", job.Name, status))
|
||||
}
|
||||
|
||||
// validateMinInterval checks that the schedule interval is not below the configured minimum.
|
||||
// It validates "every" schedules directly and estimates the interval for cron expressions.
|
||||
func (t *CronTool) validateMinInterval(schedule cron.CronSchedule) error {
|
||||
minMS := int64(t.minIntervalSeconds) * 1000
|
||||
|
||||
switch schedule.Kind {
|
||||
case "every":
|
||||
if schedule.EveryMS != nil && *schedule.EveryMS < minMS {
|
||||
return fmt.Errorf(
|
||||
"interval %ds is below the minimum allowed interval of %ds",
|
||||
*schedule.EveryMS/1000, t.minIntervalSeconds,
|
||||
)
|
||||
}
|
||||
case "cron":
|
||||
if schedule.Expr != "" {
|
||||
now := time.Now()
|
||||
next1, err := cron.NextTickAfter(schedule.Expr, now)
|
||||
if err != nil {
|
||||
return nil // let gronx validate later
|
||||
}
|
||||
next2, err := cron.NextTickAfter(schedule.Expr, next1)
|
||||
if err != nil {
|
||||
return nil
|
||||
}
|
||||
gapMS := next2.Sub(next1).Milliseconds()
|
||||
if gapMS < minMS {
|
||||
return fmt.Errorf(
|
||||
"cron expression '%s' fires every %ds, which is below the minimum allowed interval of %ds",
|
||||
schedule.Expr, gapMS/1000, t.minIntervalSeconds,
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// ExecuteJob executes a cron job through the agent
|
||||
func (t *CronTool) ExecuteJob(ctx context.Context, job *cron.CronJob) string {
|
||||
// Get channel/chatID from job payload
|
||||
|
|
|
|||
|
|
@ -12,11 +12,17 @@ import (
|
|||
)
|
||||
|
||||
func newTestCronTool(t *testing.T) *CronTool {
|
||||
t.Helper()
|
||||
return newTestCronToolWithMinInterval(t, 0)
|
||||
}
|
||||
|
||||
func newTestCronToolWithMinInterval(t *testing.T, minInterval int) *CronTool {
|
||||
t.Helper()
|
||||
storePath := filepath.Join(t.TempDir(), "cron.json")
|
||||
cronService := cron.NewCronService(storePath, nil)
|
||||
msgBus := bus.NewMessageBus()
|
||||
cfg := config.DefaultConfig()
|
||||
cfg.Tools.Cron.MinIntervalSeconds = minInterval
|
||||
tool, err := NewCronTool(cronService, nil, msgBus, t.TempDir(), true, 0, cfg)
|
||||
if err != nil {
|
||||
t.Fatalf("NewCronTool() error: %v", err)
|
||||
|
|
@ -136,3 +142,101 @@ func TestCronTool_NonCommandJobDefaultsDeliverToFalse(t *testing.T) {
|
|||
t.Fatal("expected deliver=false by default for non-command jobs")
|
||||
}
|
||||
}
|
||||
|
||||
// TestCronTool_MinInterval_EveryBelowLimit verifies that every_seconds below min_interval is rejected
|
||||
func TestCronTool_MinInterval_EveryBelowLimit(t *testing.T) {
|
||||
tool := newTestCronToolWithMinInterval(t, 60)
|
||||
ctx := WithToolContext(context.Background(), "cli", "direct")
|
||||
result := tool.Execute(ctx, map[string]any{
|
||||
"action": "add",
|
||||
"message": "too fast",
|
||||
"every_seconds": float64(5),
|
||||
})
|
||||
|
||||
if !result.IsError {
|
||||
t.Fatal("expected error for every_seconds below min_interval")
|
||||
}
|
||||
if !strings.Contains(result.ForLLM, "below the minimum allowed interval") {
|
||||
t.Errorf("expected min interval error, got: %s", result.ForLLM)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCronTool_MinInterval_EveryAboveLimit verifies that every_seconds at or above min_interval is accepted
|
||||
func TestCronTool_MinInterval_EveryAboveLimit(t *testing.T) {
|
||||
tool := newTestCronToolWithMinInterval(t, 60)
|
||||
ctx := WithToolContext(context.Background(), "cli", "direct")
|
||||
result := tool.Execute(ctx, map[string]any{
|
||||
"action": "add",
|
||||
"message": "fast enough",
|
||||
"every_seconds": float64(60),
|
||||
})
|
||||
|
||||
if result.IsError {
|
||||
t.Fatalf("expected every_seconds at min_interval to succeed, got: %s", result.ForLLM)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCronTool_MinInterval_CronExprBelowLimit verifies that cron expressions firing too frequently are rejected
|
||||
func TestCronTool_MinInterval_CronExprBelowLimit(t *testing.T) {
|
||||
tool := newTestCronToolWithMinInterval(t, 120)
|
||||
ctx := WithToolContext(context.Background(), "cli", "direct")
|
||||
// "* * * * *" fires every minute (60s), which is below 120s min
|
||||
result := tool.Execute(ctx, map[string]any{
|
||||
"action": "add",
|
||||
"message": "every minute",
|
||||
"cron_expr": "* * * * *",
|
||||
})
|
||||
|
||||
if !result.IsError {
|
||||
t.Fatal("expected error for cron expression below min_interval")
|
||||
}
|
||||
if !strings.Contains(result.ForLLM, "below the minimum allowed interval") {
|
||||
t.Errorf("expected min interval error, got: %s", result.ForLLM)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCronTool_MinInterval_CronExprAboveLimit verifies that cron expressions with sufficient intervals are accepted
|
||||
func TestCronTool_MinInterval_CronExprAboveLimit(t *testing.T) {
|
||||
tool := newTestCronToolWithMinInterval(t, 60)
|
||||
ctx := WithToolContext(context.Background(), "cli", "direct")
|
||||
// "0 * * * *" fires every hour (3600s), well above 60s min
|
||||
result := tool.Execute(ctx, map[string]any{
|
||||
"action": "add",
|
||||
"message": "every hour",
|
||||
"cron_expr": "0 * * * *",
|
||||
})
|
||||
|
||||
if result.IsError {
|
||||
t.Fatalf("expected cron expression above min_interval to succeed, got: %s", result.ForLLM)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCronTool_MinInterval_AtJobNotAffected verifies that one-time (at) jobs are not affected by min_interval
|
||||
func TestCronTool_MinInterval_AtJobNotAffected(t *testing.T) {
|
||||
tool := newTestCronToolWithMinInterval(t, 3600)
|
||||
ctx := WithToolContext(context.Background(), "cli", "direct")
|
||||
result := tool.Execute(ctx, map[string]any{
|
||||
"action": "add",
|
||||
"message": "one time only",
|
||||
"at_seconds": float64(5),
|
||||
})
|
||||
|
||||
if result.IsError {
|
||||
t.Fatalf("expected one-time at_seconds job to bypass min_interval, got: %s", result.ForLLM)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCronTool_MinInterval_ZeroDisablesCheck verifies that min_interval=0 disables the check
|
||||
func TestCronTool_MinInterval_ZeroDisablesCheck(t *testing.T) {
|
||||
tool := newTestCronToolWithMinInterval(t, 0)
|
||||
ctx := WithToolContext(context.Background(), "cli", "direct")
|
||||
result := tool.Execute(ctx, map[string]any{
|
||||
"action": "add",
|
||||
"message": "very fast",
|
||||
"every_seconds": float64(1),
|
||||
})
|
||||
|
||||
if result.IsError {
|
||||
t.Fatalf("expected every_seconds=1 to succeed when min_interval=0, got: %s", result.ForLLM)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue