mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-21 02:33:32 +00:00
Make will_fix_later an operational commitment, not silent shut-up
Before this change, all three dismissal reasons funnelled through the same "swallow re-detection at same/lower severity" path in FindingsStore.Add, so will_fix_later was functionally identical to expected_behavior — Pulse stayed quiet forever despite the dismiss_finding tool literally telling the LLM "Pulse Patrol will continue to monitor this issue." Now will_fix_later sets Finding.RemindAt (default 7 days) at dismissal time. Once RemindAt has passed, the next re-detection clears the dismissal and emits a `reminded` lifecycle event so the operator sees their lapsed commitment instead of a swallowed finding. expected_behavior keeps acknowledged-forever semantics; not_an_issue keeps Suppressed=true. The LLM tool response now surfaces the remind-at date so Patrol's conversational explanations stay aligned with the contract.
This commit is contained in:
@@ -1534,3 +1534,14 @@ The open-source/free `PUT /api/ai/patrol/autonomy` adapter may persist
|
||||
findings-only `monitor` configuration and the governed investigation budget /
|
||||
timeout clamps, but it must continue to reject `approval`, `assisted`, and
|
||||
`full` autonomy with the canonical safe-remediation license response.
|
||||
The same canonical findings store owns dismissal-reason semantics. The three
|
||||
`dismissed_reason` values must remain behaviorally distinct, not copy-only
|
||||
variants: `not_an_issue` flips `Suppressed=true`, `expected_behavior`
|
||||
acknowledges without escalation, and `will_fix_later` is an operator
|
||||
commitment that populates `Finding.RemindAt` (default
|
||||
`DefaultWillFixLaterRemindAfter`, 7 days). On re-detection, the canonical
|
||||
store wakes a `will_fix_later` finding once `RemindAt` has passed by
|
||||
clearing the dismissal and emitting a `reminded` lifecycle event, and the
|
||||
`dismiss_finding` LLM tool response must communicate the remind-at date so
|
||||
Patrol's conversational explanations stay aligned with the persisted
|
||||
behavior.
|
||||
|
||||
@@ -1075,3 +1075,15 @@ on shared helpers. Findings shells may link or format from feature-owned
|
||||
presentation helpers, but Patrol runtime severity, title cleanup, and primary
|
||||
settings actions must stay keyed to the canonical Patrol service identity
|
||||
instead of reimplementing those branches in links-only or leaf badge surfaces.
|
||||
Finding dismissal reasons now carry distinct operational semantics, not just
|
||||
copy variants. `not_an_issue` permanently suppresses (Suppressed=true);
|
||||
`expected_behavior` acknowledges the finding forever without escalation;
|
||||
`will_fix_later` is a real operational commitment — `FindingsStore.Dismiss`
|
||||
populates `Finding.RemindAt` (default `DefaultWillFixLaterRemindAfter`, 7
|
||||
days), and the next re-detection after `RemindAt` clears the dismissal and
|
||||
records a `reminded` lifecycle event so the operator sees their commitment
|
||||
lapsed instead of the finding being silently swallowed forever. Severity
|
||||
escalation still wakes any dismissed finding regardless of reason. The
|
||||
`dismiss_finding` LLM tool response surfaces the remind-at date in plain
|
||||
language so Patrol's own conversational explanations stay aligned with this
|
||||
contract.
|
||||
|
||||
+69
-9
@@ -45,6 +45,17 @@ const (
|
||||
FindingCategoryGeneral FindingCategory = "general"
|
||||
)
|
||||
|
||||
// DismissReasonWillFixLater marks a finding as "I will fix this later" — an
|
||||
// operational commitment with an implicit deadline. See Finding.RemindAt.
|
||||
const DismissReasonWillFixLater = "will_fix_later"
|
||||
|
||||
// DefaultWillFixLaterRemindAfter is how long Pulse waits before nagging the
|
||||
// operator about an open will_fix_later finding. This makes the dismissal a
|
||||
// real commitment rather than a silent shut-up: if the finding still trips
|
||||
// after this window, it surfaces again with a "reminded" lifecycle event
|
||||
// instead of being swallowed forever.
|
||||
const DefaultWillFixLaterRemindAfter = 7 * 24 * time.Hour
|
||||
|
||||
// FindingLoopState represents the current stage of the patrol detect/remediate loop.
|
||||
type FindingLoopState string
|
||||
|
||||
@@ -140,6 +151,13 @@ type Finding struct {
|
||||
UserNote string `json:"user_note,omitempty"` // Freeform user explanation, included in LLM context
|
||||
TimesRaised int `json:"times_raised"` // How many times this finding has been detected
|
||||
Suppressed bool `json:"suppressed"` // Permanently suppress similar findings for this resource
|
||||
// RemindAt turns "will_fix_later" into a real operational commitment.
|
||||
// When set, the dismissal stays quiet until this timestamp passes; after
|
||||
// it passes, the next re-detection clears the dismissal and surfaces the
|
||||
// finding again with a "reminded" lifecycle event so the operator sees
|
||||
// "you said you'd fix this on <date>, it's still happening." Only
|
||||
// populated for reason == "will_fix_later".
|
||||
RemindAt *time.Time `json:"remind_at,omitempty"`
|
||||
|
||||
// Investigation fields - tracks autonomous AI investigation of findings
|
||||
InvestigationSessionID string `json:"investigation_session_id,omitempty"` // Chat session ID if being investigated
|
||||
@@ -183,6 +201,7 @@ type findingJSON struct {
|
||||
UserNote string `json:"user_note,omitempty"`
|
||||
TimesRaised int `json:"times_raised"`
|
||||
Suppressed bool `json:"suppressed"`
|
||||
RemindAt *time.Time `json:"remind_at,omitempty"`
|
||||
InvestigationSessionID string `json:"investigation_session_id,omitempty"`
|
||||
InvestigationStatus string `json:"investigation_status,omitempty"`
|
||||
InvestigationOutcome string `json:"investigation_outcome,omitempty"`
|
||||
@@ -226,6 +245,7 @@ func (f Finding) MarshalJSON() ([]byte, error) {
|
||||
UserNote: f.UserNote,
|
||||
TimesRaised: f.TimesRaised,
|
||||
Suppressed: f.Suppressed,
|
||||
RemindAt: f.RemindAt,
|
||||
InvestigationSessionID: f.InvestigationSessionID,
|
||||
InvestigationStatus: f.InvestigationStatus,
|
||||
InvestigationOutcome: f.InvestigationOutcome,
|
||||
@@ -274,6 +294,7 @@ func (f *Finding) UnmarshalJSON(data []byte) error {
|
||||
UserNote: payload.UserNote,
|
||||
TimesRaised: payload.TimesRaised,
|
||||
Suppressed: payload.Suppressed,
|
||||
RemindAt: payload.RemindAt,
|
||||
InvestigationSessionID: payload.InvestigationSessionID,
|
||||
InvestigationStatus: payload.InvestigationStatus,
|
||||
InvestigationOutcome: payload.InvestigationOutcome,
|
||||
@@ -967,7 +988,8 @@ func (s *FindingsStore) Add(f *Finding) bool {
|
||||
}
|
||||
}
|
||||
|
||||
// Check if dismissed or suppressed - only update if severity has escalated
|
||||
// Check if dismissed or suppressed - only update if severity has escalated,
|
||||
// or if a "will_fix_later" remind-at deadline has passed.
|
||||
if existing.DismissedReason != "" || existing.Suppressed {
|
||||
severityOrder := map[FindingSeverity]int{
|
||||
FindingSeverityInfo: 0,
|
||||
@@ -975,8 +997,17 @@ func (s *FindingsStore) Add(f *Finding) bool {
|
||||
FindingSeverityWarning: 2,
|
||||
FindingSeverityCritical: 3,
|
||||
}
|
||||
// If new severity is same or lower, don't reactivate
|
||||
if severityOrder[f.Severity] <= severityOrder[existing.Severity] {
|
||||
// "will_fix_later" with a passed RemindAt is a real operator
|
||||
// commitment that has expired — surface the finding again. This
|
||||
// is the only case where a same-or-lower-severity re-detection
|
||||
// reactivates a dismissed finding; "expected_behavior" stays
|
||||
// quiet, "not_an_issue" stays suppressed.
|
||||
willFixReminderDue := existing.DismissedReason == DismissReasonWillFixLater &&
|
||||
existing.RemindAt != nil &&
|
||||
time.Now().After(*existing.RemindAt) &&
|
||||
!existing.Suppressed
|
||||
if !willFixReminderDue && severityOrder[f.Severity] <= severityOrder[existing.Severity] {
|
||||
// New severity is same or lower and there's no reminder due — don't reactivate.
|
||||
existing.LastSeenAt = time.Now()
|
||||
existing.TimesRaised++
|
||||
s.appendLifecycleLocked(existing, "seen_while_suppressed", "Re-detected while dismissed/suppressed with non-escalated severity", existing.LoopState, existing.LoopState, nil)
|
||||
@@ -984,10 +1015,23 @@ func (s *FindingsStore) Add(f *Finding) bool {
|
||||
s.scheduleSave()
|
||||
return false
|
||||
}
|
||||
// Severity escalated - clear dismissal/suppression and reactivate
|
||||
if willFixReminderDue {
|
||||
prevRemindAt := existing.RemindAt
|
||||
meta := map[string]string{"reason_was": existing.DismissedReason}
|
||||
if prevRemindAt != nil {
|
||||
meta["remind_at"] = prevRemindAt.Format(time.RFC3339)
|
||||
}
|
||||
s.appendLifecycleLocked(existing, "reminded", "will_fix_later remind-at deadline passed; re-surfacing finding", string(FindingLoopStateDismissed), string(FindingLoopStateDetected), meta)
|
||||
}
|
||||
// Severity escalated or remind-at fired - clear dismissal/suppression and reactivate.
|
||||
existing.DismissedReason = ""
|
||||
existing.Suppressed = false
|
||||
existing.UserNote = "" // Clear note since situation changed
|
||||
existing.RemindAt = nil
|
||||
if !willFixReminderDue {
|
||||
// On severity escalation we drop the operator's note (situation changed);
|
||||
// on a remind-at wake we keep it because the operator's promise is still relevant context.
|
||||
existing.UserNote = ""
|
||||
}
|
||||
existing.AcknowledgedAt = nil
|
||||
}
|
||||
|
||||
@@ -1236,20 +1280,35 @@ func (s *FindingsStore) Dismiss(id, reason, note string) bool {
|
||||
// Mark as acknowledged for all dismiss reasons
|
||||
now := time.Now()
|
||||
f.AcknowledgedAt = &now
|
||||
s.appendLifecycleLocked(f, "dismissed", "", f.LoopState, string(FindingLoopStateDismissed), map[string]string{
|
||||
"reason": reason,
|
||||
})
|
||||
dismissMeta := map[string]string{"reason": reason}
|
||||
|
||||
// Only "not_an_issue" creates permanent suppression
|
||||
// This is for true false positives where the detection logic is wrong
|
||||
if reason == "not_an_issue" {
|
||||
f.Suppressed = true
|
||||
f.RemindAt = nil
|
||||
}
|
||||
// "will_fix_later" is an operational commitment, not a permanent silence.
|
||||
// Default a 7-day remind-at so the next re-detection after the window
|
||||
// surfaces the finding with a "reminded" lifecycle event instead of being
|
||||
// swallowed forever like "expected_behavior".
|
||||
if reason == DismissReasonWillFixLater {
|
||||
remindAt := now.Add(DefaultWillFixLaterRemindAfter)
|
||||
f.RemindAt = &remindAt
|
||||
dismissMeta["remind_at"] = remindAt.Format(time.RFC3339)
|
||||
} else if reason != "" {
|
||||
f.RemindAt = nil
|
||||
}
|
||||
s.appendLifecycleLocked(f, "dismissed", "", f.LoopState, string(FindingLoopStateDismissed), dismissMeta)
|
||||
s.syncLoopStateLocked(f)
|
||||
// For "expected_behavior" and "will_fix_later":
|
||||
// For "expected_behavior":
|
||||
// - Finding stays visible (not suppressed, not snoozed)
|
||||
// - But is marked as dismissed/acknowledged so user knows they've reviewed it
|
||||
// - Severity escalation will clear DismissedReason and reactivate
|
||||
// For "will_fix_later":
|
||||
// - Same as expected_behavior, plus RemindAt is set; the next re-detection
|
||||
// after RemindAt clears the dismissal and the finding surfaces again
|
||||
// with a "reminded" lifecycle event.
|
||||
|
||||
s.mu.Unlock()
|
||||
s.scheduleSave()
|
||||
@@ -1278,6 +1337,7 @@ func (s *FindingsStore) Undismiss(id string) bool {
|
||||
f.DismissedReason = ""
|
||||
f.Suppressed = false
|
||||
f.AcknowledgedAt = nil
|
||||
f.RemindAt = nil
|
||||
s.appendLifecycleLocked(f, "undismissed", "", string(FindingLoopStateDismissed), string(FindingLoopStateDetected), nil)
|
||||
// Keep UserNote in case user wants to see their notes
|
||||
|
||||
|
||||
@@ -1426,3 +1426,186 @@ func TestFindingsStore_Dismiss_DifferentReasons(t *testing.T) {
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestFindingsStore_Dismiss_WillFixLater_DefaultsRemindAt(t *testing.T) {
|
||||
// will_fix_later is meant to be an operational commitment, not a silent
|
||||
// shut-up. Dismiss must populate RemindAt so the finding can wake itself
|
||||
// after the deadline; expected_behavior must NOT populate it (that's
|
||||
// the "acknowledged forever" semantic), and not_an_issue must clear it
|
||||
// (that's the "permanent suppression" semantic).
|
||||
t.Run("will_fix_later sets RemindAt ~7 days out", func(t *testing.T) {
|
||||
store := NewFindingsStore()
|
||||
f := &Finding{
|
||||
ID: "f-wfl",
|
||||
ResourceID: "res-1",
|
||||
Severity: FindingSeverityWarning,
|
||||
Title: "Disk pressure",
|
||||
}
|
||||
store.Add(f)
|
||||
|
||||
before := time.Now()
|
||||
store.Dismiss("f-wfl", DismissReasonWillFixLater, "Plan to upgrade Q3")
|
||||
after := time.Now()
|
||||
|
||||
got := store.Get("f-wfl")
|
||||
if got.RemindAt == nil {
|
||||
t.Fatal("Expected RemindAt to be set for will_fix_later")
|
||||
}
|
||||
expectedMin := before.Add(DefaultWillFixLaterRemindAfter)
|
||||
expectedMax := after.Add(DefaultWillFixLaterRemindAfter)
|
||||
if got.RemindAt.Before(expectedMin) || got.RemindAt.After(expectedMax) {
|
||||
t.Errorf("RemindAt should be ~7 days out, got %v (expected window %v..%v)", got.RemindAt, expectedMin, expectedMax)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("expected_behavior does not set RemindAt", func(t *testing.T) {
|
||||
store := NewFindingsStore()
|
||||
store.Add(&Finding{ID: "f-eb", ResourceID: "res-2", Severity: FindingSeverityWarning})
|
||||
store.Dismiss("f-eb", "expected_behavior", "Known")
|
||||
if got := store.Get("f-eb"); got.RemindAt != nil {
|
||||
t.Errorf("expected_behavior must not set RemindAt; got %v", got.RemindAt)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("not_an_issue clears RemindAt", func(t *testing.T) {
|
||||
store := NewFindingsStore()
|
||||
store.Add(&Finding{ID: "f-nai", ResourceID: "res-3", Severity: FindingSeverityWarning})
|
||||
// Pre-stage a stale RemindAt to confirm not_an_issue clears it.
|
||||
store.Dismiss("f-nai", DismissReasonWillFixLater, "later")
|
||||
if store.Get("f-nai").RemindAt == nil {
|
||||
t.Fatal("setup: RemindAt should have been set by will_fix_later")
|
||||
}
|
||||
store.Undismiss("f-nai")
|
||||
store.Dismiss("f-nai", "not_an_issue", "false positive")
|
||||
if got := store.Get("f-nai"); got.RemindAt != nil {
|
||||
t.Errorf("not_an_issue must clear RemindAt; got %v", got.RemindAt)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("Undismiss clears RemindAt", func(t *testing.T) {
|
||||
store := NewFindingsStore()
|
||||
store.Add(&Finding{ID: "f-un", ResourceID: "res-4", Severity: FindingSeverityWarning})
|
||||
store.Dismiss("f-un", DismissReasonWillFixLater, "later")
|
||||
store.Undismiss("f-un")
|
||||
if got := store.Get("f-un"); got.RemindAt != nil {
|
||||
t.Errorf("Undismiss must clear RemindAt; got %v", got.RemindAt)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestFindingsStore_Add_WillFixLater_StaysQuietBeforeRemindAt(t *testing.T) {
|
||||
// While the will_fix_later remind-at is still in the future, re-detection
|
||||
// at same severity must NOT reactivate the finding — the operator's
|
||||
// commitment is still "honored" and Pulse stays quiet.
|
||||
store := NewFindingsStore()
|
||||
store.Add(&Finding{
|
||||
ID: "f-quiet",
|
||||
ResourceID: "res-1",
|
||||
Severity: FindingSeverityWarning,
|
||||
Title: "Disk pressure",
|
||||
})
|
||||
store.Dismiss("f-quiet", DismissReasonWillFixLater, "Q3 upgrade")
|
||||
|
||||
// Re-detect at same severity while RemindAt is still in the future.
|
||||
store.Add(&Finding{
|
||||
ID: "f-quiet",
|
||||
ResourceID: "res-1",
|
||||
Severity: FindingSeverityWarning,
|
||||
Title: "Disk pressure (again)",
|
||||
})
|
||||
|
||||
got := store.Get("f-quiet")
|
||||
if !got.IsDismissed() {
|
||||
t.Error("Finding must stay dismissed before RemindAt fires")
|
||||
}
|
||||
if got.DismissedReason != DismissReasonWillFixLater {
|
||||
t.Errorf("DismissedReason should still be will_fix_later, got %s", got.DismissedReason)
|
||||
}
|
||||
if got.TimesRaised < 1 {
|
||||
t.Error("TimesRaised must still increment to track recurrences while quiet")
|
||||
}
|
||||
}
|
||||
|
||||
func TestFindingsStore_Add_WillFixLater_WakesAfterRemindAt(t *testing.T) {
|
||||
// Once the will_fix_later remind-at has passed, the next re-detection
|
||||
// must clear the dismissal and surface the finding again with a
|
||||
// "reminded" lifecycle event so the operator sees their commitment lapsed.
|
||||
store := NewFindingsStore()
|
||||
store.Add(&Finding{
|
||||
ID: "f-wake",
|
||||
ResourceID: "res-1",
|
||||
Severity: FindingSeverityWarning,
|
||||
Title: "Disk pressure",
|
||||
})
|
||||
store.Dismiss("f-wake", DismissReasonWillFixLater, "Q3 upgrade")
|
||||
|
||||
// Backdate RemindAt so it has already passed.
|
||||
store.mu.Lock()
|
||||
past := time.Now().Add(-1 * time.Hour)
|
||||
store.findings["f-wake"].RemindAt = &past
|
||||
store.mu.Unlock()
|
||||
|
||||
// Re-detect at same severity — should now wake.
|
||||
store.Add(&Finding{
|
||||
ID: "f-wake",
|
||||
ResourceID: "res-1",
|
||||
Severity: FindingSeverityWarning,
|
||||
Title: "Disk pressure (still)",
|
||||
})
|
||||
|
||||
got := store.Get("f-wake")
|
||||
if got.IsDismissed() {
|
||||
t.Error("Finding must be un-dismissed once RemindAt has passed")
|
||||
}
|
||||
if got.DismissedReason != "" {
|
||||
t.Errorf("DismissedReason must be cleared after wake, got %s", got.DismissedReason)
|
||||
}
|
||||
if got.RemindAt != nil {
|
||||
t.Errorf("RemindAt must be cleared after wake, got %v", got.RemindAt)
|
||||
}
|
||||
// UserNote should be preserved on remind-at wake (operator's promise is still relevant context).
|
||||
if got.UserNote != "Q3 upgrade" {
|
||||
t.Errorf("UserNote must be preserved across remind-at wake; got %q", got.UserNote)
|
||||
}
|
||||
// Lifecycle must record the "reminded" event so the operator sees their commitment lapsed.
|
||||
foundReminded := false
|
||||
for _, ev := range got.Lifecycle {
|
||||
if ev.Type == "reminded" {
|
||||
foundReminded = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !foundReminded {
|
||||
t.Error("Lifecycle must contain a 'reminded' event after will_fix_later wake")
|
||||
}
|
||||
}
|
||||
|
||||
func TestFinding_RemindAt_RoundTripsThroughJSON(t *testing.T) {
|
||||
// RemindAt must persist across save/load so the operator's commitment
|
||||
// survives a process restart.
|
||||
when := time.Now().Add(48 * time.Hour).UTC().Truncate(time.Second)
|
||||
original := Finding{
|
||||
ID: "f-json",
|
||||
ResourceID: "res-1",
|
||||
Severity: FindingSeverityWarning,
|
||||
DismissedReason: DismissReasonWillFixLater,
|
||||
RemindAt: &when,
|
||||
}
|
||||
bytes, err := original.MarshalJSON()
|
||||
if err != nil {
|
||||
t.Fatalf("MarshalJSON: %v", err)
|
||||
}
|
||||
if !strings.Contains(string(bytes), "\"remind_at\"") {
|
||||
t.Errorf("MarshalJSON output should contain remind_at field, got %s", string(bytes))
|
||||
}
|
||||
var roundTripped Finding
|
||||
if err := roundTripped.UnmarshalJSON(bytes); err != nil {
|
||||
t.Fatalf("UnmarshalJSON: %v", err)
|
||||
}
|
||||
if roundTripped.RemindAt == nil {
|
||||
t.Fatal("RemindAt must round-trip through JSON")
|
||||
}
|
||||
if !roundTripped.RemindAt.Equal(when) {
|
||||
t.Errorf("RemindAt round-trip mismatch: got %v want %v", roundTripped.RemindAt, when)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3383,7 +3383,8 @@ func (s *Service) executeTool(ctx context.Context, req ExecuteRequest, tc provid
|
||||
// Format a helpful response based on reason
|
||||
var resultMsg string
|
||||
if reason == "will_fix_later" {
|
||||
resultMsg = fmt.Sprintf("Finding dismissed as '%s'. Pulse Patrol will continue to monitor this issue.\nID: %s\nNote: %s", reason, findingID, note)
|
||||
remindAt := time.Now().Add(DefaultWillFixLaterRemindAfter).Format("2006-01-02")
|
||||
resultMsg = fmt.Sprintf("Finding dismissed as '%s'. Pulse Patrol will stay quiet on this until %s; if the finding is still tripping after that date, it will surface again with a 'reminded' lifecycle event.\nID: %s\nNote: %s", reason, remindAt, findingID, note)
|
||||
} else {
|
||||
resultMsg = fmt.Sprintf("Finding dismissed as '%s' and suppression rule created. Similar findings for this resource will not be raised again.\nID: %s\nNote: %s", reason, findingID, note)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user