From 6113a16ba94b21f68315596ce57222f8716f99b5 Mon Sep 17 00:00:00 2001
From: Eric Jacksch <securityguy@users.noreply.github.com>
Date: Fri, 20 Mar 2026 11:43:27 -0400
Subject: [PATCH] fix(cron): reload store on external file change; only save
 when state changes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two related bugs caused cron jobs added via the CLI to be silently lost
while picoclaw was running:

1. checkJobs() called saveStoreUnsafe() unconditionally every second,
   overwriting the file with the running service's in-memory state. Any
   job written by the CLI was clobbered within at most one tick.

2. The running service never reloaded the store from disk, so CLI-added
   jobs were invisible in memory and would never execute even if the file
   was not overwritten.

Fix: track the store file's mtime in fileModTime (updated after every
load and save). At the start of each checkJobs() tick, stat the file; if
its mtime is newer than fileModTime, reload from disk. Only call
saveStoreUnsafe() when there are actually due jobs — no state changes
means no write, which stops the clobbering entirely.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 pkg/cron/service.go | 64 +++++++++++++++++++++++++++++++--------------
 1 file changed, 44 insertions(+), 20 deletions(-)

diff --git a/pkg/cron/service.go b/pkg/cron/service.go
index 04775ac42..47878eb27 100644
--- a/pkg/cron/service.go
+++ b/pkg/cron/service.go
@@ -59,13 +59,14 @@ 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{}
-	gronx     *gronx.Gronx
+	storePath   string
+	store       *CronStore
+	onJob       JobHandler
+	mu          sync.RWMutex
+	running     bool
+	stopChan    chan struct{}
+	gronx       *gronx.Gronx
+	fileModTime time.Time // mtime of store file at last load or save
 }
 
 func NewCronService(storePath string, onJob JobHandler) *CronService {
@@ -140,6 +141,13 @@ func (cs *CronService) checkJobs() {
 		return
 	}
 
+	// Reload from disk if the file was modified externally (e.g. by the CLI).
+	if info, err := os.Stat(cs.storePath); err == nil && info.ModTime().After(cs.fileModTime) {
+		if err := cs.loadStore(); err != nil {
+			log.Printf("[cron] failed to reload store: %v", err)
+		}
+	}
+
 	now := time.Now().UnixMilli()
 	var dueJobIDs []string
 
@@ -151,19 +159,23 @@ func (cs *CronService) checkJobs() {
 		}
 	}
 
-	// Reset next run for due jobs before unlocking to avoid duplicate execution.
-	dueMap := make(map[string]bool, len(dueJobIDs))
-	for _, jobID := range dueJobIDs {
-		dueMap[jobID] = true
-	}
-	for i := range cs.store.Jobs {
-		if dueMap[cs.store.Jobs[i].ID] {
-			cs.store.Jobs[i].State.NextRunAtMS = nil
+	// Only persist state when there are due jobs to avoid clobbering
+	// concurrent writes from the CLI or other processes.
+	if len(dueJobIDs) > 0 {
+		// Reset next run before unlocking to prevent duplicate execution.
+		dueMap := make(map[string]bool, len(dueJobIDs))
+		for _, jobID := range dueJobIDs {
+			dueMap[jobID] = true
+		}
+		for i := range cs.store.Jobs {
+			if dueMap[cs.store.Jobs[i].ID] {
+				cs.store.Jobs[i].State.NextRunAtMS = nil
+			}
 		}
-	}
 
-	if err := cs.saveStoreUnsafe(); err != nil {
-		log.Printf("[cron] failed to save store: %v", err)
+		if err := cs.saveStoreUnsafe(); err != nil {
+			log.Printf("[cron] failed to save store: %v", err)
+		}
 	}
 
 	cs.mu.Unlock()
@@ -347,7 +359,13 @@ func (cs *CronService) loadStore() error {
 		return err
 	}
 
-	return json.Unmarshal(data, cs.store)
+	if err := json.Unmarshal(data, cs.store); err != nil {
+		return err
+	}
+	if info, err := os.Stat(cs.storePath); err == nil {
+		cs.fileModTime = info.ModTime()
+	}
+	return nil
 }
 
 func (cs *CronService) saveStoreUnsafe() error {
@@ -357,7 +375,13 @@ func (cs *CronService) saveStoreUnsafe() error {
 	}
 
 	// Use unified atomic write utility with explicit sync for flash storage reliability.
-	return fileutil.WriteFileAtomic(cs.storePath, data, 0o600)
+	if err := fileutil.WriteFileAtomic(cs.storePath, data, 0o600); err != nil {
+		return err
+	}
+	if info, err := os.Stat(cs.storePath); err == nil {
+		cs.fileModTime = info.ModTime()
+	}
+	return nil
 }
 
 func (cs *CronService) AddJob(