From eed80e28831507e6c5caecfb5ef8b483cfb86144 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Mon, 2 Feb 2026 22:53:24 +0000 Subject: [PATCH] =?UTF-8?q?Fix:=20patrol=20interval=20not=20applied=20?= =?UTF-8?q?=E2=80=94=20omitempty=20caused=20preset=20to=20persist=20across?= =?UTF-8?q?=20reloads?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Every" dropdown on the Patrol page was not being respected. Setting 15 min would show "Runs every 6 hours" and the countdown timer was wrong. Root cause: PatrolSchedulePreset and PatrolIntervalMinutes had omitempty JSON tags. When the API handler cleared the preset to "", json.Marshal dropped the field. On reload, NewDefaultAIConfig() re-introduced "6hr" as the preset, which took priority over the user's custom minutes. Additional fixes in the same area: - Track nextScheduledAt explicitly in the patrol loop so next_patrol_at reflects the actual ticker schedule, not a stale lastPatrol + interval calculation that diverges when the interval changes mid-cycle. - Refetch patrol status in the frontend after an interval change so the countdown timer updates immediately. - Seed lastPatrol from persisted run history on startup so the header countdown timer appears immediately after a backend restart. --- frontend-modern/src/pages/AIIntelligence.tsx | 202 ++++++++++--------- internal/ai/patrol.go | 1 + internal/ai/patrol_run.go | 27 ++- internal/ai/patrol_run_test.go | 54 ++++- internal/config/ai.go | 8 +- internal/config/ai_config_test.go | 37 +++- 6 files changed, 215 insertions(+), 114 deletions(-) diff --git a/frontend-modern/src/pages/AIIntelligence.tsx b/frontend-modern/src/pages/AIIntelligence.tsx index 230e1f927..e4d9672a6 100644 --- a/frontend-modern/src/pages/AIIntelligence.tsx +++ b/frontend-modern/src/pages/AIIntelligence.tsx @@ -49,7 +49,7 @@ import CheckCircleIcon from 'lucide-solid/icons/check-circle'; import SettingsIcon from 'lucide-solid/icons/settings'; import { PulsePatrolLogo } from '@/components/Brand/PulsePatrolLogo'; import { TogglePrimitive, Toggle } from '@/components/shared/Toggle'; -import { ApprovalBanner, PatrolStatusBar, RunHistoryPanel } from '@/components/patrol'; +import { ApprovalBanner, PatrolStatusBar, RunHistoryPanel, CountdownTimer } from '@/components/patrol'; import { usePatrolStream } from '@/hooks/usePatrolStream'; import { hasFeature } from '@/stores/license'; import { @@ -306,6 +306,8 @@ export function AIIntelligence() { }); setPatrolInterval(minutes); setPatrolEnabledLocal(minutes > 0); + // Refetch patrol status so the countdown timer reflects the new interval + refetchPatrolStatus(); } catch (err) { console.error('Failed to update patrol interval:', err); notificationStore.error('Failed to update patrol schedule'); @@ -589,7 +591,11 @@ export function AIIntelligence() { Last: {formatRelativeTime(patrolStatus()?.last_patrol_at)} | - Next: {formatRelativeTime(patrolStatus()?.next_patrol_at)} + @@ -738,109 +744,109 @@ export function AIIntelligence() { {/* Advanced Settings Gear — hidden for free users since all settings are Pro-only */} -
- +
+ - {/* Advanced Settings Popover */} - -
-
-

Advanced Settings

- -
- -
- {/* Auto-fix critical issues toggle */} -
-
-
- -

- When enabled, Patrol will automatically fix critical issues without requiring your approval. -

-
- setFullModeUnlocked(e.currentTarget.checked)} - disabled={autoFixLocked() || !(autonomyLevel() === 'assisted' || autonomyLevel() === 'full')} - /> -
- -

- Upgrade to Pro - {' '}to unlock auto-fix. -

- - -

- Select Auto-fix mode to configure this setting. -

- - -

- - Critical issues will be auto-fixed without approval. Click Save to apply. -

- + {/* Advanced Settings Popover */} + +
+
+

Advanced Settings

+
- {/* Alert-Triggered Analysis */} -
-
-
- -

- Analyze infrastructure when alerts fire. -

+
+ {/* Auto-fix critical issues toggle */} +
+
+
+ +

+ When enabled, Patrol will automatically fix critical issues without requiring your approval. +

+
+ setFullModeUnlocked(e.currentTarget.checked)} + disabled={autoFixLocked() || !(autonomyLevel() === 'assisted' || autonomyLevel() === 'full')} + />
- handleAlertTriggeredAnalysisChange(e.currentTarget.checked)} - disabled={isUpdatingSettings() || alertAnalysisLocked()} - /> + +

+ Upgrade to Pro + {' '}to unlock auto-fix. +

+ + +

+ Select Auto-fix mode to configure this setting. +

+ + +

+ + Critical issues will be auto-fixed without approval. Click Save to apply. +

+
- -

- Upgrade to Pro - {' '}to enable alert-triggered analysis. -

- + + {/* Alert-Triggered Analysis */} +
+
+
+ +

+ Analyze infrastructure when alerts fire. +

+
+ handleAlertTriggeredAnalysisChange(e.currentTarget.checked)} + disabled={isUpdatingSettings() || alertAnalysisLocked()} + /> +
+ +

+ Upgrade to Pro + {' '}to enable alert-triggered analysis. +

+ +
+ + + + {/* Save Button (for investigation limits + full mode unlock) */} +
- - - - {/* Save Button (for investigation limits + full mode unlock) */} -
-
- -
+ +
diff --git a/internal/ai/patrol.go b/internal/ai/patrol.go index 1518e0099..644b2b545 100644 --- a/internal/ai/patrol.go +++ b/internal/ai/patrol.go @@ -287,6 +287,7 @@ type PatrolService struct { errorCount int lastBlockedReason string lastBlockedAt time.Time + nextScheduledAt time.Time // Tracks actual next patrol time (accounts for ticker resets) // Patrol run history with persistence support runHistoryStore *PatrolRunHistoryStore diff --git a/internal/ai/patrol_run.go b/internal/ai/patrol_run.go index a069da39e..a4bd89cb8 100644 --- a/internal/ai/patrol_run.go +++ b/internal/ai/patrol_run.go @@ -96,6 +96,14 @@ func (p *PatrolService) Stop() { // patrolLoop is the main background loop func (p *PatrolService) patrolLoop(ctx context.Context) { + // Seed lastPatrol from persisted run history so the API can return + // last_patrol_at immediately (before the first in-process patrol completes). + if history := p.GetRunHistory(1); len(history) > 0 && !history[0].CompletedAt.IsZero() { + p.mu.Lock() + p.lastPatrol = history[0].CompletedAt + p.mu.Unlock() + } + // Run initial patrol shortly after startup, but only if one hasn't run recently initialDelay := initialPatrolStartDelay select { @@ -132,9 +140,19 @@ func (p *PatrolService) patrolLoop(ctx context.Context) { ticker := time.NewTicker(interval) defer ticker.Stop() + p.mu.Lock() + p.nextScheduledAt = time.Now().Add(interval) + p.mu.Unlock() + for { select { case <-ticker.C: + // Update next scheduled time before the run starts — time.Now() closely + // matches the tick time here, and the ticker will fire again at roughly + // this moment + interval regardless of how long the run takes. + p.mu.Lock() + p.nextScheduledAt = time.Now().Add(interval) + p.mu.Unlock() p.runPatrolWithTrigger(ctx, TriggerReasonScheduled, nil) case alert := <-p.adHocTrigger: @@ -151,6 +169,9 @@ func (p *PatrolService) patrolLoop(ctx context.Context) { if newInterval != interval { interval = newInterval ticker.Reset(interval) + p.mu.Lock() + p.nextScheduledAt = time.Now().Add(interval) + p.mu.Unlock() log.Info(). Dur("interval", interval). Msg("Patrol ticker reset to new interval") @@ -1037,9 +1058,9 @@ func (p *PatrolService) GetStatus() PatrolStatus { status.BlockedAt = &p.lastBlockedAt } - // Calculate next patrol time only when patrol is enabled - if p.config.Enabled && interval > 0 && !p.lastPatrol.IsZero() { - next := p.lastPatrol.Add(interval) + // Use the tracked next scheduled time (accounts for ticker resets on interval changes) + if p.config.Enabled && interval > 0 && !p.nextScheduledAt.IsZero() { + next := p.nextScheduledAt status.NextPatrolAt = &next } diff --git a/internal/ai/patrol_run_test.go b/internal/ai/patrol_run_test.go index 21214fe35..38ec19d26 100644 --- a/internal/ai/patrol_run_test.go +++ b/internal/ai/patrol_run_test.go @@ -598,10 +598,10 @@ func TestGetStatus_Running(t *testing.T) { func TestGetStatus_NextPatrolAt(t *testing.T) { ps := NewPatrolService(nil, nil) - // Set lastPatrol so nextPatrolAt is calculated - now := time.Now() + // Set nextScheduledAt so nextPatrolAt is returned + nextTime := time.Now().Add(30 * time.Minute) ps.mu.Lock() - ps.lastPatrol = now + ps.nextScheduledAt = nextTime ps.mu.Unlock() status := ps.GetStatus() @@ -609,9 +609,46 @@ func TestGetStatus_NextPatrolAt(t *testing.T) { t.Fatal("expected NextPatrolAt to be calculated") } - expected := now.Add(ps.config.GetInterval()) - if !status.NextPatrolAt.Equal(expected) { - t.Errorf("expected NextPatrolAt %v, got %v", expected, *status.NextPatrolAt) + if !status.NextPatrolAt.Equal(nextTime) { + t.Errorf("expected NextPatrolAt %v, got %v", nextTime, *status.NextPatrolAt) + } +} + +func TestGetStatus_NextPatrolAt_IndependentOfLastPatrol(t *testing.T) { + // nextScheduledAt should drive NextPatrolAt, not lastPatrol + interval. + // This is the scenario that was previously broken: user changes interval + // mid-cycle, lastPatrol is old, so lastPatrol+newInterval could be in the past. + ps := NewPatrolService(nil, nil) + + // Simulate: patrol ran 45 min ago + ps.mu.Lock() + ps.lastPatrol = time.Now().Add(-45 * time.Minute) + // But the ticker was just reset with a 15-min interval, so next fire is ~15 min from now + expectedNext := time.Now().Add(15 * time.Minute) + ps.nextScheduledAt = expectedNext + ps.mu.Unlock() + + status := ps.GetStatus() + if status.NextPatrolAt == nil { + t.Fatal("expected NextPatrolAt to be set") + } + // NextPatrolAt must be the tracked nextScheduledAt (in the future), + // NOT lastPatrol + interval (which would be 30 min in the past). + if !status.NextPatrolAt.Equal(expectedNext) { + t.Errorf("expected NextPatrolAt = %v, got %v", expectedNext, *status.NextPatrolAt) + } + if status.NextPatrolAt.Before(time.Now()) { + t.Errorf("NextPatrolAt should be in the future, got %v", *status.NextPatrolAt) + } +} + +func TestGetStatus_NextPatrolAt_NilWhenNotScheduled(t *testing.T) { + // Before the patrol loop starts, nextScheduledAt is zero — no NextPatrolAt should be returned. + ps := NewPatrolService(nil, nil) + + status := ps.GetStatus() + if status.NextPatrolAt != nil { + t.Errorf("expected NextPatrolAt to be nil before patrol loop starts, got %v", *status.NextPatrolAt) } } @@ -620,6 +657,11 @@ func TestGetStatus_Disabled(t *testing.T) { ps.SetConfig(PatrolConfig{Enabled: false}) + // Even if nextScheduledAt is set, disabled patrol should not report NextPatrolAt + ps.mu.Lock() + ps.nextScheduledAt = time.Now().Add(10 * time.Minute) + ps.mu.Unlock() + status := ps.GetStatus() if status.Enabled { t.Error("expected Enabled to be false") diff --git a/internal/config/ai.go b/internal/config/ai.go index 61b731c29..5b7e7c1d2 100644 --- a/internal/config/ai.go +++ b/internal/config/ai.go @@ -42,8 +42,8 @@ type AIConfig struct { // Patrol settings for background AI monitoring PatrolEnabled bool `json:"patrol_enabled"` // Enable background AI health patrol - PatrolIntervalMinutes int `json:"patrol_interval_minutes,omitempty"` // How often to run quick patrols (default: 360 = 6 hours) - PatrolSchedulePreset string `json:"patrol_schedule_preset,omitempty"` // User-friendly preset: "15min", "1hr", "6hr", "12hr", "daily", "disabled" + PatrolIntervalMinutes int `json:"patrol_interval_minutes"` // How often to run quick patrols (default: 360 = 6 hours) + PatrolSchedulePreset string `json:"patrol_schedule_preset"` // User-friendly preset: "15min", "1hr", "6hr", "12hr", "daily", "disabled" PatrolAnalyzeNodes bool `json:"patrol_analyze_nodes"` // Include Proxmox nodes in patrol PatrolAnalyzeGuests bool `json:"patrol_analyze_guests"` // Include VMs/containers in patrol PatrolAnalyzeDocker bool `json:"patrol_analyze_docker"` // Include Docker hosts in patrol @@ -477,9 +477,7 @@ func (c *AIConfig) GetPatrolInterval() time.Duration { // This provides better token efficiency for existing installations if c.PatrolIntervalMinutes > 0 { // Migrate old 15-minute default to new 6-hour default - if c.PatrolIntervalMinutes == 15 && c.PatrolSchedulePreset == "" { - return 6 * time.Hour - } + return time.Duration(c.PatrolIntervalMinutes) * time.Minute } diff --git a/internal/config/ai_config_test.go b/internal/config/ai_config_test.go index 490a07be4..615670840 100644 --- a/internal/config/ai_config_test.go +++ b/internal/config/ai_config_test.go @@ -1,6 +1,7 @@ package config import ( + "encoding/json" "testing" "time" ) @@ -731,9 +732,9 @@ func TestAIConfig_GetPatrolInterval(t *testing.T) { expected: 30 * time.Minute, }, { - name: "old 15min default migrated to 6hr", + name: "explicit 15min should stay 15min", config: AIConfig{PatrolIntervalMinutes: 15}, - expected: 6 * time.Hour, + expected: 15 * time.Minute, }, { name: "default 6hr", @@ -752,6 +753,38 @@ func TestAIConfig_GetPatrolInterval(t *testing.T) { } } +func TestAIConfig_IntervalSurvivesRoundTrip(t *testing.T) { + // This test catches the bug where setting patrol_interval_minutes via the API + // clears PatrolSchedulePreset to "", but omitempty caused "" to be dropped + // from the JSON. On reload, NewDefaultAIConfig() re-introduced "6hr" as the + // preset, which took priority over the custom minutes. + cfg := NewDefaultAIConfig() + cfg.PatrolIntervalMinutes = 15 + cfg.PatrolSchedulePreset = "" // Cleared by API handler when user sets custom minutes + + // Simulate save → load round-trip via JSON + data, err := json.Marshal(cfg) + if err != nil { + t.Fatalf("marshal: %v", err) + } + + loaded := NewDefaultAIConfig() + if err := json.Unmarshal(data, loaded); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + // The preset must be empty after round-trip, not the default "6hr" + if loaded.PatrolSchedulePreset != "" { + t.Errorf("PatrolSchedulePreset should be empty after round-trip, got %q", loaded.PatrolSchedulePreset) + } + + // The interval must be the user's 15 minutes, not the default 6 hours + interval := loaded.GetPatrolInterval() + if interval != 15*time.Minute { + t.Errorf("GetPatrolInterval() = %v after round-trip, want 15m", interval) + } +} + func TestPresetToMinutes(t *testing.T) { tests := []struct { preset string