From 6266d0a838da071b7ea93e1ebb8923e2b3b8d9a0 Mon Sep 17 00:00:00 2001 From: Rahul Bansal Date: Tue, 3 Mar 2026 19:09:49 +0530 Subject: [PATCH] feat: scope cron list/manage operations to current chat target (#995) --- pkg/cron/service.go | 72 ++++++++++++++++++++++++++++++++++++++++ pkg/cron/service_test.go | 35 +++++++++++++++++++ pkg/tools/cron.go | 33 ++++++++++++++++-- 3 files changed, 137 insertions(+), 3 deletions(-) diff --git a/pkg/cron/service.go b/pkg/cron/service.go index 6962041c1..ed1bd95fd 100644 --- a/pkg/cron/service.go +++ b/pkg/cron/service.go @@ -7,6 +7,7 @@ import ( "fmt" "log" "os" + "strings" "sync" "time" @@ -450,6 +451,77 @@ func (cs *CronService) EnableJob(jobID string, enabled bool) *CronJob { return nil } + +func targetMatches(job *CronJob, channel, to string) bool { + return strings.EqualFold(strings.TrimSpace(job.Payload.Channel), strings.TrimSpace(channel)) && + strings.EqualFold(strings.TrimSpace(job.Payload.To), strings.TrimSpace(to)) +} + +// ListJobsForTarget returns jobs for a specific channel+recipient. +func (cs *CronService) ListJobsForTarget(channel, to string, includeDisabled bool) []CronJob { + cs.mu.RLock() + defer cs.mu.RUnlock() + + filtered := make([]CronJob, 0) + for _, job := range cs.store.Jobs { + if !targetMatches(&job, channel, to) { + continue + } + if !includeDisabled && !job.Enabled { + continue + } + filtered = append(filtered, job) + } + return filtered +} + +// RemoveJobForTarget removes a job only when it belongs to the given channel+recipient. +func (cs *CronService) RemoveJobForTarget(jobID, channel, to string) bool { + cs.mu.Lock() + defer cs.mu.Unlock() + + for _, job := range cs.store.Jobs { + if job.ID == jobID { + if !targetMatches(&job, channel, to) { + return false + } + return cs.removeJobUnsafe(jobID) + } + } + return false +} + +// EnableJobForTarget toggles a job only when it belongs to the given channel+recipient. +func (cs *CronService) EnableJobForTarget(jobID, channel, to string, enabled bool) *CronJob { + cs.mu.Lock() + defer cs.mu.Unlock() + + for i := range cs.store.Jobs { + job := &cs.store.Jobs[i] + if job.ID != jobID { + continue + } + if !targetMatches(job, channel, to) { + return nil + } + job.Enabled = enabled + job.UpdatedAtMS = time.Now().UnixMilli() + + if enabled { + job.State.NextRunAtMS = cs.computeNextRun(&job.Schedule, time.Now().UnixMilli()) + } else { + job.State.NextRunAtMS = nil + } + + if err := cs.saveStoreUnsafe(); err != nil { + log.Printf("[cron] failed to save store after enable: %v", err) + } + return job + } + + return nil +} + func (cs *CronService) ListJobs(includeDisabled bool) []CronJob { cs.mu.RLock() defer cs.mu.RUnlock() diff --git a/pkg/cron/service_test.go b/pkg/cron/service_test.go index 1a0dd1829..84e065553 100644 --- a/pkg/cron/service_test.go +++ b/pkg/cron/service_test.go @@ -36,3 +36,38 @@ func TestSaveStore_FilePermissions(t *testing.T) { func int64Ptr(v int64) *int64 { return &v } + +func TestScopedCronOperations(t *testing.T) { + tmpDir := t.TempDir() + storePath := filepath.Join(tmpDir, "cron", "jobs.json") + cs := NewCronService(storePath, nil) + + every := int64(60000) + j1, err := cs.AddJob("u1", CronSchedule{Kind: "every", EveryMS: &every}, "m1", false, "telegram", "user1") + if err != nil { + t.Fatalf("AddJob u1 failed: %v", err) + } + j2, err := cs.AddJob("u2", CronSchedule{Kind: "every", EveryMS: &every}, "m2", false, "telegram", "user2") + if err != nil { + t.Fatalf("AddJob u2 failed: %v", err) + } + + jobsU1 := cs.ListJobsForTarget("telegram", "user1", true) + if len(jobsU1) != 1 || jobsU1[0].ID != j1.ID { + t.Fatalf("expected only user1 job, got %+v", jobsU1) + } + + if cs.RemoveJobForTarget(j2.ID, "telegram", "user1") { + t.Fatalf("expected remove to fail across target boundary") + } + if !cs.RemoveJobForTarget(j2.ID, "telegram", "user2") { + t.Fatalf("expected remove to succeed for owner target") + } + + if job := cs.EnableJobForTarget(j1.ID, "telegram", "user2", false); job != nil { + t.Fatalf("expected enable/disable to fail across target boundary") + } + if job := cs.EnableJobForTarget(j1.ID, "telegram", "user1", false); job == nil || job.Enabled { + t.Fatalf("expected owner disable to succeed") + } +} diff --git a/pkg/tools/cron.go b/pkg/tools/cron.go index 6888d1326..b52d9c8a7 100644 --- a/pkg/tools/cron.go +++ b/pkg/tools/cron.go @@ -217,7 +217,16 @@ func (t *CronTool) addJob(args map[string]any) *ToolResult { } func (t *CronTool) listJobs() *ToolResult { - jobs := t.cronService.ListJobs(false) + t.mu.RLock() + channel := t.channel + chatID := t.chatID + t.mu.RUnlock() + + if channel == "" || chatID == "" { + return ErrorResult("no session context (channel/chat_id not set). Use this tool in an active conversation.") + } + + jobs := t.cronService.ListJobsForTarget(channel, chatID, false) if len(jobs) == 0 { return SilentResult("No scheduled jobs") @@ -243,24 +252,42 @@ func (t *CronTool) listJobs() *ToolResult { } func (t *CronTool) removeJob(args map[string]any) *ToolResult { + t.mu.RLock() + channel := t.channel + chatID := t.chatID + t.mu.RUnlock() + + if channel == "" || chatID == "" { + return ErrorResult("no session context (channel/chat_id not set). Use this tool in an active conversation.") + } + jobID, ok := args["job_id"].(string) if !ok || jobID == "" { return ErrorResult("job_id is required for remove") } - if t.cronService.RemoveJob(jobID) { + if t.cronService.RemoveJobForTarget(jobID, channel, chatID) { return SilentResult(fmt.Sprintf("Cron job removed: %s", jobID)) } return ErrorResult(fmt.Sprintf("Job %s not found", jobID)) } func (t *CronTool) enableJob(args map[string]any, enable bool) *ToolResult { + t.mu.RLock() + channel := t.channel + chatID := t.chatID + t.mu.RUnlock() + + if channel == "" || chatID == "" { + return ErrorResult("no session context (channel/chat_id not set). Use this tool in an active conversation.") + } + jobID, ok := args["job_id"].(string) if !ok || jobID == "" { return ErrorResult("job_id is required for enable/disable") } - job := t.cronService.EnableJob(jobID, enable) + job := t.cronService.EnableJobForTarget(jobID, channel, chatID, enable) if job == nil { return ErrorResult(fmt.Sprintf("Job %s not found", jobID)) }