diff --git a/frontend-modern/src/components/shared/AIOverviewTable.tsx b/frontend-modern/src/components/shared/AIOverviewTable.tsx index 4daa6d7..c40377a 100644 --- a/frontend-modern/src/components/shared/AIOverviewTable.tsx +++ b/frontend-modern/src/components/shared/AIOverviewTable.tsx @@ -168,49 +168,46 @@ export const AIOverviewTable: Component<{ showWhenEmpty?: boolean }> = (props) = }); } - // Remediations - ONLY show high-value entries: - // 1. Achievements logged via log_achievement tool (have summary field or [achievement:] action) - // 2. Records with explicit summary field (new backend generates these) - // Skip: Low-value diagnostic commands without summaries - const achievements = remediations().filter(rem => { - // Has an AI-generated summary - always show - if (rem.summary && rem.summary.trim() !== '') return true; - // Is an explicit achievement logged via the tool - if (rem.action.startsWith('[achievement:')) return true; - // Skip everything else - no more generic "Checked disk usage" entries - return false; - }); + // Remediations - ONLY show actual ACTIONS (restarts, resizes, cleanups, fixes) + // Skip diagnostic commands (df, grep, cat, tail, ps) - they don't provide lasting value + const isActionableCommand = (action: string): boolean => { + const cmd = action.trim().replace(/^\[[^\]]+\]\s*/, ''); // Strip [host] prefix + const actionPatterns = [ + 'docker restart', 'docker start', 'docker stop', 'docker rm', + 'docker compose up', 'docker compose down', 'docker compose restart', + 'systemctl restart', 'systemctl start', 'systemctl stop', 'systemctl enable', 'systemctl disable', + 'service restart', 'service start', 'service stop', + 'pct resize', 'pct start', 'pct stop', 'pct shutdown', 'pct reboot', + 'qm resize', 'qm start', 'qm stop', 'qm shutdown', 'qm reboot', + 'rm -', 'rm /', + 'chmod', 'chown', + 'mkdir', + 'mv ', 'cp ', + 'apt install', 'apt upgrade', 'apt remove', + 'yum install', 'dnf install', + 'pip install', 'npm install', + 'kill ', 'pkill ', 'killall ', + 'reboot', 'shutdown', + ]; + return actionPatterns.some(pattern => cmd.includes(pattern)); + }; - for (const rem of achievements) { - const categoryBadgeClass: Record = { - diagnosis: 'bg-blue-100 text-blue-700 dark:bg-blue-900/40 dark:text-blue-300', - fix: 'bg-green-100 text-green-700 dark:bg-green-900/40 dark:text-green-300', - verification: 'bg-purple-100 text-purple-700 dark:bg-purple-900/40 dark:text-purple-300', - discovery: 'bg-cyan-100 text-cyan-700 dark:bg-cyan-900/40 dark:text-cyan-300', - optimization: 'bg-amber-100 text-amber-700 dark:bg-amber-900/40 dark:text-amber-300', - }; + const actionableRemediations = remediations().filter(rem => isActionableCommand(rem.action)); - // Extract category from [achievement:category] action - const categoryMatch = rem.action.match(/\[achievement:(\w+)\]/); - const category = categoryMatch ? categoryMatch[1] : 'general'; - - // Format category for display - const categoryLabels: Record = { - diagnosis: 'Diagnosed', - fix: 'Fixed', - verification: 'Verified', - discovery: 'Discovered', - optimization: 'Optimized', - general: 'Helped', + for (const rem of actionableRemediations) { + const outcomeBadgeClass: Record = { + resolved: 'bg-green-100 text-green-700 dark:bg-green-900/40 dark:text-green-300', + partial: 'bg-amber-100 text-amber-700 dark:bg-amber-900/40 dark:text-amber-300', + failed: 'bg-red-100 text-red-700 dark:bg-red-900/40 dark:text-red-300', }; rows.push({ id: `rem-${rem.finding_id}-${rem.timestamp}`, type: 'impact', - typeBadge: categoryLabels[category] || 'Helped', - typeBadgeClass: categoryBadgeClass[category] || 'bg-emerald-100 text-emerald-700 dark:bg-emerald-900/40 dark:text-emerald-300', - title: rem.summary || rem.problem, // Summary is the key field - subtitle: rem.resource_name || '', // Show resource name if available + typeBadge: rem.outcome === 'resolved' ? 'Fixed' : rem.outcome === 'failed' ? 'Failed' : 'Action', + typeBadgeClass: outcomeBadgeClass[rem.outcome] || 'bg-gray-100 text-gray-600 dark:bg-gray-800 dark:text-gray-300', + title: rem.summary || summarizeValue(rem.problem, rem.action), // Use backend summary or fallback + subtitle: rem.resource_name || '', timestamp: formatRelativeTime(rem.timestamp), locked: false, badgeClass: 'text-emerald-600 dark:text-emerald-400', diff --git a/internal/ai/service.go b/internal/ai/service.go index 820f3bc..3c990a4 100644 --- a/internal/ai/service.go +++ b/internal/ai/service.go @@ -1097,7 +1097,6 @@ You have access to tools to execute commands on the target system. You should: 2. Actually execute the commands - don't just explain what commands to run 3. For Proxmox operations (resize disk, manage containers/VMs), run commands on the HOST (target_type=host) 4. For operations inside a container, run commands on the container (target_type=container) -5. **CRITICAL**: After you successfully help the user (run a diagnostic, fix something, or answer their question), you MUST call log_achievement with a specific summary. This is required and helps remind users of Pulse Pro's value. Examples of actions you can perform: - Resize LXC disk: pct resize rootfs +10G (run on host) @@ -1105,13 +1104,7 @@ Examples of actions you can perform: - View processes: ps aux --sort=-%mem | head -20 - Check logs: tail -100 /var/log/syslog -Always execute the commands rather than telling the user how to do it. - -**MANDATORY**: After completing any task, you MUST call log_achievement. Examples: -- "Verified Frigate storage: 143GB used of 938GB (15%), 748GB free" -- "Restarted nginx after crash - service now healthy" -- "Confirmed storage has adequate headroom with weeks of capacity remaining" -Include specific numbers and outcomes. Do NOT skip this step.` +Always execute the commands rather than telling the user how to do it.` } // Inject previously learned knowledge about this guest @@ -1306,7 +1299,6 @@ You have access to tools to execute commands on the target system. You should: 2. Actually execute the commands - don't just explain what commands to run 3. For Proxmox operations (resize disk, manage containers/VMs), run commands on the HOST (target_type=host) 4. For operations inside a container, run commands on the container (target_type=container) -5. **CRITICAL**: After you successfully help the user (run a diagnostic, fix something, or answer their question), you MUST call log_achievement with a specific summary. This is required and helps remind users of Pulse Pro's value. Examples of actions you can perform: - Resize LXC disk: pct resize rootfs +10G (run on host) @@ -1314,13 +1306,7 @@ Examples of actions you can perform: - View processes: ps aux --sort=-%mem | head -20 - Check logs: tail -100 /var/log/syslog -Always execute the commands rather than telling the user how to do it. - -**MANDATORY**: After completing any task, you MUST call log_achievement. Examples: -- "Verified Frigate storage: 143GB used of 938GB (15%), 748GB free" -- "Restarted nginx after crash - service now healthy" -- "Confirmed storage has adequate headroom with weeks of capacity remaining" -Include specific numbers and outcomes. Do NOT skip this step.` +Always execute the commands rather than telling the user how to do it.` } // Inject previously learned knowledge about this guest @@ -1654,7 +1640,18 @@ func (s *Service) getToolInputDisplay(tc providers.ToolCall) string { // logRemediation logs a tool execution to the remediation log for operational memory // This enables learning from past fix attempts +// ONLY logs commands that perform actual ACTIONS (restarts, resizes, cleanups, fixes) +// Skips diagnostic commands (df, grep, cat, tail, ps) to avoid noise func (s *Service) logRemediation(req ExecuteRequest, command, output string, success bool) { + // First, check if this is an actionable command worth logging + // Diagnostic/read-only commands don't provide value in the achievement log + if !isActionableCommand(command) { + log.Debug(). + Str("command", command). + Msg("Skipping diagnostic command - not logging to remediation history") + return + } + s.mu.RLock() patrol := s.patrolService s.mu.RUnlock() @@ -1714,12 +1711,55 @@ func (s *Service) logRemediation(req ExecuteRequest, command, output string, suc Automatic: req.UseCase == "patrol", // Patrol runs are automatic }) - log.Debug(). + log.Info(). Str("resource_id", req.TargetID). + Str("resource_name", resourceName). Str("command", command). Str("summary", summary). Bool("success", success). - Msg("Logged remediation action to operational memory") + Msg("Logged ACTION to Pulse AI Impact") +} + +// isActionableCommand returns true if the command performs an actual action +// that's worth logging as an achievement, false for read-only diagnostics +func isActionableCommand(cmd string) bool { + cmd = strings.TrimSpace(cmd) + + // Strip [host] or [hostname] prefix if present + if strings.HasPrefix(cmd, "[") { + if idx := strings.Index(cmd, "]"); idx != -1 { + cmd = strings.TrimSpace(cmd[idx+1:]) + } + } + + // Commands that PERFORM ACTIONS (should be logged) + actionPatterns := []string{ + "docker restart", "docker start", "docker stop", "docker rm", + "docker compose up", "docker compose down", "docker compose restart", + "systemctl restart", "systemctl start", "systemctl stop", "systemctl enable", "systemctl disable", + "service restart", "service start", "service stop", + "pct resize", "pct start", "pct stop", "pct shutdown", "pct reboot", + "qm resize", "qm start", "qm stop", "qm shutdown", "qm reboot", + "rm -", "rm /", // File deletion/cleanup + "chmod", "chown", // Permission fixes + "mkdir", // Creating directories + "mv ", "cp ", // File operations + "echo >", "tee ", // Writing to files + "apt install", "apt upgrade", "apt remove", + "yum install", "dnf install", + "pip install", "npm install", + "kill ", "pkill ", "killall ", + "reboot", "shutdown", + } + + for _, pattern := range actionPatterns { + if strings.Contains(cmd, pattern) { + return true + } + } + + // Everything else is diagnostic (df, grep, cat, tail, ps, ls, etc.) + return false } // generateRemediationSummary creates a human-readable summary of what a command achieved @@ -2046,25 +2086,6 @@ func (s *Service) getTools() []providers.Tool { "required": []string{"finding_id", "resolution_note"}, }, }, - { - Name: "log_achievement", - Description: "Log a meaningful achievement to remind the user what Pulse Pro accomplished for them. Call this when you've successfully helped the user with something valuable - like diagnosing an issue, fixing a problem, or answering their question with actionable information. Write a clear, specific summary that will be useful to the user in 2 weeks when they review what Pulse Pro has done for them. Examples: 'Verified Frigate is retaining 7 days of recordings with 45GB storage used', 'Restarted nginx service after detecting crash - service now healthy', 'Confirmed disk has 120GB free after cleanup'.", - InputSchema: map[string]interface{}{ - "type": "object", - "properties": map[string]interface{}{ - "achievement": map[string]interface{}{ - "type": "string", - "description": "A clear, specific summary of what was accomplished. Be specific with numbers, names, and outcomes. This will be shown to the user as a reminder of value delivered.", - }, - "category": map[string]interface{}{ - "type": "string", - "description": "Category of the achievement", - "enum": []string{"diagnosis", "fix", "verification", "discovery", "optimization"}, - }, - }, - "required": []string{"achievement"}, - }, - }, } // Add web search tool for Anthropic provider @@ -2268,68 +2289,6 @@ func (s *Service) executeTool(ctx context.Context, req ExecuteRequest, tc provid execution.Success = true return execution.Output, execution - case "log_achievement": - achievement, _ := tc.Input["achievement"].(string) - category, _ := tc.Input["category"].(string) - execution.Input = fmt.Sprintf("achievement: %s, category: %s", achievement, category) - - if achievement == "" { - execution.Output = "Error: achievement is required. Please describe what was accomplished." - return execution.Output, execution - } - - if category == "" { - category = "general" // Default category - } - - // Log the achievement to the remediation log - s.mu.RLock() - patrol := s.patrolService - s.mu.RUnlock() - - if patrol == nil { - execution.Output = "Error: Patrol service not available" - return execution.Output, execution - } - - remLog := patrol.GetRemediationLog() - if remLog == nil { - execution.Output = "Error: Remediation log not available" - return execution.Output, execution - } - - // Get resource name from context if available - resourceName := "" - if req.Context != nil { - if name, ok := req.Context["name"].(string); ok { - resourceName = name - } - } - - // Log as an achievement - use Summary field prominently, Action as "achievement" - remLog.Log(RemediationRecord{ - ResourceID: req.TargetID, - ResourceType: req.TargetType, - ResourceName: resourceName, - FindingID: req.FindingID, - Problem: req.Prompt, // Keep original prompt for context - Summary: achievement, // THIS is the key field - user-facing summary - Action: "[achievement:" + category + "]", // Mark as achievement, not a command - Outcome: OutcomeResolved, - Automatic: req.UseCase == "patrol", - }) - - log.Info(). - Str("resource_id", req.TargetID). - Str("resource_name", resourceName). - Str("category", category). - Str("achievement", achievement). - Msg("Logged AI achievement to operational memory") - - execution.Output = fmt.Sprintf("Achievement logged! This will be shown to the user as a reminder of what Pulse Pro accomplished.\nAchievement: %s\nCategory: %s", achievement, category) - execution.Success = true - return execution.Output, execution - default: execution.Output = fmt.Sprintf("Unknown tool: %s", tc.Name) return execution.Output, execution diff --git a/internal/alerts/alerts.go b/internal/alerts/alerts.go index 62d526e..514dca7 100644 --- a/internal/alerts/alerts.go +++ b/internal/alerts/alerts.go @@ -1015,10 +1015,19 @@ func (m *Manager) UpdateConfig(config AlertConfig) { } // normalizeStorageDefaults ensures storage default thresholds are set +// Trigger=0 is allowed and means "disable storage alerting" func normalizeStorageDefaults(config *AlertConfig) { - if config.StorageDefault.Trigger <= 0 { + if config.StorageDefault.Trigger < 0 { config.StorageDefault.Trigger = 85 config.StorageDefault.Clear = 80 + } else if config.StorageDefault.Trigger == 0 { + // Trigger=0 means disabled, set Clear=0 too + config.StorageDefault.Clear = 0 + } else if config.StorageDefault.Clear <= 0 { + config.StorageDefault.Clear = config.StorageDefault.Trigger - 5 + if config.StorageDefault.Clear < 0 { + config.StorageDefault.Clear = 0 + } } } @@ -1182,10 +1191,14 @@ func normalizeBackupDefaults(config *AlertConfig) { } // normalizeNodeDefaults ensures node threshold defaults exist +// Trigger=0 is allowed for Temperature and means "disable temperature alerting" func normalizeNodeDefaults(config *AlertConfig) { // Ensure temperature defaults exist for nodes so high temps alert out of the box - if config.NodeDefaults.Temperature == nil || config.NodeDefaults.Temperature.Trigger <= 0 { + if config.NodeDefaults.Temperature == nil || config.NodeDefaults.Temperature.Trigger < 0 { config.NodeDefaults.Temperature = &HysteresisThreshold{Trigger: 80, Clear: 75} + } else if config.NodeDefaults.Temperature.Trigger == 0 { + // Trigger=0 means disabled, set Clear=0 too + config.NodeDefaults.Temperature.Clear = 0 } else if config.NodeDefaults.Temperature.Clear <= 0 { config.NodeDefaults.Temperature.Clear = config.NodeDefaults.Temperature.Trigger - 5 if config.NodeDefaults.Temperature.Clear <= 0 { diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go index dc863ba..d0b04fd 100644 --- a/internal/alerts/alerts_test.go +++ b/internal/alerts/alerts_test.go @@ -1492,6 +1492,221 @@ func TestNormalizeHostDefaultsPreservesZeroTrigger(t *testing.T) { }) } +// TestNormalizeStorageDefaultsPreservesZeroTrigger verifies that setting +// StorageDefault threshold to 0 is preserved to disable storage alerting. +func TestNormalizeStorageDefaultsPreservesZeroTrigger(t *testing.T) { + t.Parallel() + + t.Run("negative trigger gets factory defaults", func(t *testing.T) { + t.Parallel() + m := newTestManager(t) + + cfg := AlertConfig{ + Enabled: true, + StorageDefault: HysteresisThreshold{Trigger: -1, Clear: 0}, + } + + m.UpdateConfig(cfg) + + m.mu.RLock() + defer m.mu.RUnlock() + + if m.config.StorageDefault.Trigger != 85 { + t.Errorf("StorageDefault trigger = %v, want 85", m.config.StorageDefault.Trigger) + } + if m.config.StorageDefault.Clear != 80 { + t.Errorf("StorageDefault clear = %v, want 80", m.config.StorageDefault.Clear) + } + }) + + t.Run("Trigger=0 preserved to disable storage alerting", func(t *testing.T) { + t.Parallel() + m := newTestManager(t) + + cfg := AlertConfig{ + Enabled: true, + StorageDefault: HysteresisThreshold{Trigger: 0, Clear: 0}, + } + + m.UpdateConfig(cfg) + + m.mu.RLock() + defer m.mu.RUnlock() + + if m.config.StorageDefault.Trigger != 0 { + t.Errorf("StorageDefault trigger = %v, want 0 (disabled)", m.config.StorageDefault.Trigger) + } + if m.config.StorageDefault.Clear != 0 { + t.Errorf("StorageDefault clear = %v, want 0 (disabled)", m.config.StorageDefault.Clear) + } + }) + + t.Run("missing Clear computed from Trigger", func(t *testing.T) { + t.Parallel() + m := newTestManager(t) + + cfg := AlertConfig{ + Enabled: true, + StorageDefault: HysteresisThreshold{Trigger: 90, Clear: 0}, + } + + m.UpdateConfig(cfg) + + m.mu.RLock() + defer m.mu.RUnlock() + + if m.config.StorageDefault.Trigger != 90 { + t.Errorf("StorageDefault trigger = %v, want 90", m.config.StorageDefault.Trigger) + } + if m.config.StorageDefault.Clear != 85 { + t.Errorf("StorageDefault clear = %v, want 85 (trigger - 5)", m.config.StorageDefault.Clear) + } + }) +} + +// TestNormalizeNodeDefaultsTemperaturePreservesZeroTrigger verifies that setting +// NodeDefaults.Temperature threshold to 0 is preserved to disable temperature alerting. +func TestNormalizeNodeDefaultsTemperaturePreservesZeroTrigger(t *testing.T) { + t.Parallel() + + t.Run("nil Temperature gets factory defaults", func(t *testing.T) { + t.Parallel() + m := newTestManager(t) + + cfg := AlertConfig{ + Enabled: true, + NodeDefaults: ThresholdConfig{}, // Empty - Temperature needs defaults + } + + m.UpdateConfig(cfg) + + m.mu.RLock() + defer m.mu.RUnlock() + + if m.config.NodeDefaults.Temperature == nil { + t.Fatal("Temperature defaults should be set") + } + if m.config.NodeDefaults.Temperature.Trigger != 80 { + t.Errorf("Temperature trigger = %v, want 80", m.config.NodeDefaults.Temperature.Trigger) + } + if m.config.NodeDefaults.Temperature.Clear != 75 { + t.Errorf("Temperature clear = %v, want 75", m.config.NodeDefaults.Temperature.Clear) + } + }) + + t.Run("Trigger=0 preserved to disable temperature alerting", func(t *testing.T) { + t.Parallel() + m := newTestManager(t) + + cfg := AlertConfig{ + Enabled: true, + NodeDefaults: ThresholdConfig{ + Temperature: &HysteresisThreshold{Trigger: 0, Clear: 0}, + }, + } + + m.UpdateConfig(cfg) + + m.mu.RLock() + defer m.mu.RUnlock() + + if m.config.NodeDefaults.Temperature == nil { + t.Fatal("Temperature should be preserved (not nil)") + } + if m.config.NodeDefaults.Temperature.Trigger != 0 { + t.Errorf("Temperature trigger = %v, want 0 (disabled)", m.config.NodeDefaults.Temperature.Trigger) + } + if m.config.NodeDefaults.Temperature.Clear != 0 { + t.Errorf("Temperature clear = %v, want 0 (disabled)", m.config.NodeDefaults.Temperature.Clear) + } + }) + + t.Run("missing Clear computed from Trigger", func(t *testing.T) { + t.Parallel() + m := newTestManager(t) + + cfg := AlertConfig{ + Enabled: true, + NodeDefaults: ThresholdConfig{ + Temperature: &HysteresisThreshold{Trigger: 85, Clear: 0}, + }, + } + + m.UpdateConfig(cfg) + + m.mu.RLock() + defer m.mu.RUnlock() + + if m.config.NodeDefaults.Temperature.Trigger != 85 { + t.Errorf("Temperature trigger = %v, want 85", m.config.NodeDefaults.Temperature.Trigger) + } + if m.config.NodeDefaults.Temperature.Clear != 80 { + t.Errorf("Temperature clear = %v, want 80 (trigger - 5)", m.config.NodeDefaults.Temperature.Clear) + } + }) +} + +// TestNormalizeDockerThresholdPreservesZeroTrigger verifies that Docker +// container thresholds can be set to 0 to disable alerting. +func TestNormalizeDockerThresholdPreservesZeroTrigger(t *testing.T) { + t.Parallel() + + t.Run("Trigger=0 disables Docker CPU alerting", func(t *testing.T) { + t.Parallel() + m := newTestManager(t) + + cfg := AlertConfig{ + Enabled: true, + DockerDefaults: DockerThresholdConfig{ + CPU: HysteresisThreshold{Trigger: 0, Clear: 0}, + Memory: HysteresisThreshold{Trigger: 85, Clear: 80}, + Disk: HysteresisThreshold{Trigger: 85, Clear: 80}, + }, + } + + m.UpdateConfig(cfg) + + m.mu.RLock() + defer m.mu.RUnlock() + + if m.config.DockerDefaults.CPU.Trigger != 0 { + t.Errorf("Docker CPU trigger = %v, want 0 (disabled)", m.config.DockerDefaults.CPU.Trigger) + } + if m.config.DockerDefaults.Memory.Trigger != 85 { + t.Errorf("Docker Memory trigger = %v, want 85", m.config.DockerDefaults.Memory.Trigger) + } + }) + + t.Run("negative trigger replaced with defaults", func(t *testing.T) { + t.Parallel() + m := newTestManager(t) + + cfg := AlertConfig{ + Enabled: true, + DockerDefaults: DockerThresholdConfig{ + CPU: HysteresisThreshold{Trigger: -5, Clear: 0}, + Memory: HysteresisThreshold{Trigger: -10, Clear: 0}, + Disk: HysteresisThreshold{Trigger: -1, Clear: 0}, + }, + } + + m.UpdateConfig(cfg) + + m.mu.RLock() + defer m.mu.RUnlock() + + if m.config.DockerDefaults.CPU.Trigger != 80 { + t.Errorf("Docker CPU trigger = %v, want 80 (default)", m.config.DockerDefaults.CPU.Trigger) + } + if m.config.DockerDefaults.Memory.Trigger != 85 { + t.Errorf("Docker Memory trigger = %v, want 85 (default)", m.config.DockerDefaults.Memory.Trigger) + } + if m.config.DockerDefaults.Disk.Trigger != 85 { + t.Errorf("Docker Disk trigger = %v, want 85 (default)", m.config.DockerDefaults.Disk.Trigger) + } + }) +} + func TestNormalizeDockerIgnoredPrefixes(t *testing.T) { t.Parallel() diff --git a/internal/config/persistence.go b/internal/config/persistence.go index 9007775..05f1714 100644 --- a/internal/config/persistence.go +++ b/internal/config/persistence.go @@ -206,9 +206,17 @@ func (c *ConfigPersistence) SaveAlertConfig(config alerts.AlertConfig) error { defer c.mu.Unlock() // Ensure critical defaults are set before saving - if config.StorageDefault.Trigger <= 0 { + // Storage: Allow Trigger=0 to disable storage alerting + if config.StorageDefault.Trigger < 0 { config.StorageDefault.Trigger = 85 config.StorageDefault.Clear = 80 + } else if config.StorageDefault.Trigger == 0 { + config.StorageDefault.Clear = 0 + } else if config.StorageDefault.Clear <= 0 { + config.StorageDefault.Clear = config.StorageDefault.Trigger - 5 + if config.StorageDefault.Clear < 0 { + config.StorageDefault.Clear = 0 + } } if config.MinimumDelta <= 0 { margin := config.HysteresisMargin @@ -409,9 +417,17 @@ func (c *ConfigPersistence) LoadAlertConfig() (*alerts.AlertConfig, error) { if string(data) == "{}" { config.Enabled = true } - if config.StorageDefault.Trigger <= 0 { + // Storage: Allow Trigger=0 to disable storage alerting + if config.StorageDefault.Trigger < 0 { config.StorageDefault.Trigger = 85 config.StorageDefault.Clear = 80 + } else if config.StorageDefault.Trigger == 0 { + config.StorageDefault.Clear = 0 + } else if config.StorageDefault.Clear <= 0 { + config.StorageDefault.Clear = config.StorageDefault.Trigger - 5 + if config.StorageDefault.Clear < 0 { + config.StorageDefault.Clear = 0 + } } if config.MinimumDelta <= 0 { config.MinimumDelta = 2.0 @@ -422,8 +438,16 @@ func (c *ConfigPersistence) LoadAlertConfig() (*alerts.AlertConfig, error) { if config.HysteresisMargin <= 0 { config.HysteresisMargin = 5.0 } - if config.NodeDefaults.Temperature == nil || config.NodeDefaults.Temperature.Trigger <= 0 { + // NodeDefaults.Temperature: Allow Trigger=0 to disable temperature alerting + if config.NodeDefaults.Temperature == nil || config.NodeDefaults.Temperature.Trigger < 0 { config.NodeDefaults.Temperature = &alerts.HysteresisThreshold{Trigger: 80, Clear: 75} + } else if config.NodeDefaults.Temperature.Trigger == 0 { + config.NodeDefaults.Temperature.Clear = 0 + } else if config.NodeDefaults.Temperature.Clear <= 0 { + config.NodeDefaults.Temperature.Clear = config.NodeDefaults.Temperature.Trigger - 5 + if config.NodeDefaults.Temperature.Clear <= 0 { + config.NodeDefaults.Temperature.Clear = 75 + } } // Host Defaults: Allow Trigger=0 to disable specific alerts if config.HostDefaults.CPU == nil || config.HostDefaults.CPU.Trigger < 0 { diff --git a/internal/config/persistence_test.go b/internal/config/persistence_test.go index 238c697..8eb11b2 100644 --- a/internal/config/persistence_test.go +++ b/internal/config/persistence_test.go @@ -233,6 +233,128 @@ func TestSaveAlertConfig_HostDefaultsZeroDisablesAlerting(t *testing.T) { } } +// TestSaveAlertConfig_StorageDefaultZeroDisablesAlerting verifies that setting +// StorageDefault threshold to 0 is preserved to disable storage alerting. +func TestSaveAlertConfig_StorageDefaultZeroDisablesAlerting(t *testing.T) { + tempDir := t.TempDir() + cp := config.NewConfigPersistence(tempDir) + if err := cp.EnsureConfigDir(); err != nil { + t.Fatalf("EnsureConfigDir: %v", err) + } + + // Config with StorageDefault.Trigger=0 to disable storage alerting + cfg := alerts.AlertConfig{ + Enabled: true, + StorageDefault: alerts.HysteresisThreshold{Trigger: 0, Clear: 0}, + } + + if err := cp.SaveAlertConfig(cfg); err != nil { + t.Fatalf("SaveAlertConfig: %v", err) + } + + loaded, err := cp.LoadAlertConfig() + if err != nil { + t.Fatalf("LoadAlertConfig: %v", err) + } + + // Storage threshold should remain at 0 (disabled), not reset to default + if loaded.StorageDefault.Trigger != 0 { + t.Errorf("StorageDefault trigger = %v, want 0 (disabled)", loaded.StorageDefault.Trigger) + } + if loaded.StorageDefault.Clear != 0 { + t.Errorf("StorageDefault clear = %v, want 0 (disabled)", loaded.StorageDefault.Clear) + } +} + +// TestSaveAlertConfig_NodeTemperatureZeroDisablesAlerting verifies that setting +// NodeDefaults.Temperature threshold to 0 is preserved to disable temperature alerting. +func TestSaveAlertConfig_NodeTemperatureZeroDisablesAlerting(t *testing.T) { + tempDir := t.TempDir() + cp := config.NewConfigPersistence(tempDir) + if err := cp.EnsureConfigDir(); err != nil { + t.Fatalf("EnsureConfigDir: %v", err) + } + + // Config with Temperature=0 to disable temperature alerting + cfg := alerts.AlertConfig{ + Enabled: true, + StorageDefault: alerts.HysteresisThreshold{Trigger: 85, Clear: 80}, + NodeDefaults: alerts.ThresholdConfig{ + Temperature: &alerts.HysteresisThreshold{Trigger: 0, Clear: 0}, + }, + } + + if err := cp.SaveAlertConfig(cfg); err != nil { + t.Fatalf("SaveAlertConfig: %v", err) + } + + loaded, err := cp.LoadAlertConfig() + if err != nil { + t.Fatalf("LoadAlertConfig: %v", err) + } + + // Temperature threshold should remain at 0 (disabled), not reset to default + if loaded.NodeDefaults.Temperature == nil { + t.Fatal("Temperature should be preserved (not nil)") + } + if loaded.NodeDefaults.Temperature.Trigger != 0 { + t.Errorf("Temperature trigger = %v, want 0 (disabled)", loaded.NodeDefaults.Temperature.Trigger) + } + if loaded.NodeDefaults.Temperature.Clear != 0 { + t.Errorf("Temperature clear = %v, want 0 (disabled)", loaded.NodeDefaults.Temperature.Clear) + } +} + +// TestSaveAlertConfig_AllThresholdsZeroDisablesAlerting is a comprehensive test +// verifying that all threshold types can be set to 0 to disable alerting. +func TestSaveAlertConfig_AllThresholdsZeroDisablesAlerting(t *testing.T) { + tempDir := t.TempDir() + cp := config.NewConfigPersistence(tempDir) + if err := cp.EnsureConfigDir(); err != nil { + t.Fatalf("EnsureConfigDir: %v", err) + } + + // Config with all thresholds set to 0 to disable all alerting + cfg := alerts.AlertConfig{ + Enabled: true, + StorageDefault: alerts.HysteresisThreshold{Trigger: 0, Clear: 0}, + NodeDefaults: alerts.ThresholdConfig{ + Temperature: &alerts.HysteresisThreshold{Trigger: 0, Clear: 0}, + }, + HostDefaults: alerts.ThresholdConfig{ + CPU: &alerts.HysteresisThreshold{Trigger: 0, Clear: 0}, + Memory: &alerts.HysteresisThreshold{Trigger: 0, Clear: 0}, + Disk: &alerts.HysteresisThreshold{Trigger: 0, Clear: 0}, + }, + } + + if err := cp.SaveAlertConfig(cfg); err != nil { + t.Fatalf("SaveAlertConfig: %v", err) + } + + loaded, err := cp.LoadAlertConfig() + if err != nil { + t.Fatalf("LoadAlertConfig: %v", err) + } + + // All thresholds should remain at 0 + if loaded.StorageDefault.Trigger != 0 { + t.Errorf("StorageDefault trigger = %v, want 0", loaded.StorageDefault.Trigger) + } + if loaded.NodeDefaults.Temperature == nil || loaded.NodeDefaults.Temperature.Trigger != 0 { + t.Errorf("Temperature trigger = %v, want 0", loaded.NodeDefaults.Temperature) + } + if loaded.HostDefaults.CPU == nil || loaded.HostDefaults.CPU.Trigger != 0 { + t.Errorf("HostDefaults.CPU trigger = %v, want 0", loaded.HostDefaults.CPU) + } + if loaded.HostDefaults.Memory == nil || loaded.HostDefaults.Memory.Trigger != 0 { + t.Errorf("HostDefaults.Memory trigger = %v, want 0", loaded.HostDefaults.Memory) + } + if loaded.HostDefaults.Disk == nil || loaded.HostDefaults.Disk.Trigger != 0 { + t.Errorf("HostDefaults.Disk trigger = %v, want 0", loaded.HostDefaults.Disk) + } +} + func TestAlertConfigPersistenceNormalizesDockerIgnoredPrefixes(t *testing.T) { tempDir := t.TempDir() cp := config.NewConfigPersistence(tempDir) @@ -291,7 +413,8 @@ func TestLoadAlertConfigAppliesDefaults(t *testing.T) { CriticalDays: 8, }, NodeDefaults: alerts.ThresholdConfig{ - Temperature: &alerts.HysteresisThreshold{Trigger: 0, Clear: 0}, + // Use negative value to test "unset" case (negative means unset, 0 means disabled) + Temperature: &alerts.HysteresisThreshold{Trigger: -1, Clear: 0}, }, } @@ -321,6 +444,7 @@ func TestLoadAlertConfigAppliesDefaults(t *testing.T) { if loaded.NodeDefaults.Temperature == nil { t.Fatalf("expected node temperature defaults to be set") } + // Negative trigger should be replaced with defaults (80/75) if loaded.NodeDefaults.Temperature.Trigger != 80 || loaded.NodeDefaults.Temperature.Clear != 75 { t.Fatalf("expected temperature defaults 80/75, got %+v", loaded.NodeDefaults.Temperature) }