From 3e920df03d991d3837258665fbd51d62d6f6af14 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Fri, 11 Sep 2026 07:05:13 +0100 Subject: [PATCH] fix(alerts): skip unchanged pending-intent checkpoints Complete the JSON-only checkpoint fast path: compare sorted, bounded actual pending-state bytes before replacement. Preserve changes, clearing, failed-write retry and unsafe-destination repair. Focused before regression fails; repeated race proof passes. Sixty unchanged synthetic checkpoints fall from 245760 process write bytes to zero; changing alert freshness remains a separate contributor, not aggregate issue #1966 acceptance. Change-source: pulse-maintainer --- .../v6/internal/subsystems/alerts.md | 13 ++ internal/alerts/alerts_test.go | 115 ++++++++++++++++++ .../intent_checkpoint_unchanged_unix_test.go | 45 +++++++ internal/alerts/intent_persistence.go | 16 ++- 4 files changed, 188 insertions(+), 1 deletion(-) create mode 100644 internal/alerts/intent_checkpoint_unchanged_unix_test.go diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index 38cbd4a5d3..53ed99cdf3 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -2923,3 +2923,16 @@ notification enqueue atomic with alert resolution. before the former 180-minute deadline, plus current payload/routing selection, copy isolation, snooze and invalid-level rejection. These deterministic component tests establish neither installed delivery timing nor the cause of a delayed email. + +### Unchanged intent-pending checkpoints + +The serialized active-alert checkpoint also compares the complete, sorted +pending-policy JSON against a bounded regular file with mode 0600 before +replacement. Empty and nonempty unchanged pending state leave the file intact; +changed or cleared state, missing/stale content, unsafe destinations and failed +writes retain replacement/retry semantics. This does not change intent timing, +retention, alert freshness or the existing persistence durability contract. +`TestIntentCheckpoint*` exercises the complete JSON-only checkpoint without +starting database workers, including restart loading and Unix hardening. The +active mirror's unchanged fast path alone does not cover this companion file. +This is a narrow write contributor, not aggregate installed-write acceptance. diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go index bf72356b75..0cca818bb0 100644 --- a/internal/alerts/alerts_test.go +++ b/internal/alerts/alerts_test.go @@ -21322,3 +21322,118 @@ func TestActiveMirrorUnchangedRepairsDirectoryPermissions(t *testing.T) { }) } } + +// Use the complete JSON-only checkpoint, not just the active-alert mirror: +// unchanged pending-policy state must not undo the mirror's write reduction. +func TestIntentCheckpointUnchanged(t *testing.T) { + m := &Manager{alertsDir: t.TempDir()} + save := func() { + t.Helper() + if err := m.SaveActiveAlerts(); err != nil { + t.Fatal(err) + } + } + save() + path := filepath.Join(m.alertsDir, intentPendingFileName) + first, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + for i := 0; i < 20; i++ { + save() + after, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if !os.SameFile(first, after) { + t.Fatal("unchanged empty pending state was replaced") + } + } + after, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if !os.SameFile(first, after) { + t.Fatal("unchanged empty pending state was replaced") + } + now := time.Now().UTC() + m.intentPending = map[string]IntentPendingState{"a": {TrackingKey: "a", ResourceID: "r", Signal: "offline", FirstMatchedAt: now, LastObservedAt: now}} + save() + restored := &Manager{alertsDir: m.alertsDir} + if err := restored.loadIntentPendingNoLock(); err != nil { + t.Fatal(err) + } + if len(restored.intentPending) != 1 { + t.Fatal("changed pending state was not persisted") + } + first, err = os.Stat(path) + if err != nil { + t.Fatal(err) + } + save() + after, err = os.Stat(path) + if err != nil { + t.Fatal(err) + } + if !os.SameFile(first, after) { + t.Fatal("unchanged nonempty pending state was replaced") + } + // Compare actual bytes, not cached metadata: same-size stale state is repaired. + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + stale := append([]byte(nil), data...) + stale[0] = ' ' + if err := os.WriteFile(path, stale, alertsFilePerm); err != nil { + t.Fatal(err) + } + save() + got, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(got) != string(data) { + t.Fatal("stale state survived checkpoint") + } + if err := os.Rename(path, path+".saved"); err != nil { + t.Fatal(err) + } + save() + if _, err := os.Stat(path); err != nil { + t.Fatal(err) + } + m.intentPending = nil + save() + got, err = os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(got) != "[]" { + t.Fatal("cleared pending state was not persisted") + } +} + +func TestIntentCheckpointRetriesFailedWrite(t *testing.T) { + m := &Manager{alertsDir: t.TempDir()} + path := filepath.Join(m.alertsDir, intentPendingFileName) + if err := os.Mkdir(path, 0700); err != nil { + t.Fatal(err) + } + if err := m.SaveActiveAlerts(); err == nil { + t.Fatal("expected obstructed checkpoint failure") + } + if err := os.Rename(path, path+".obstruction"); err != nil { + t.Fatal(err) + } + if err := m.SaveActiveAlerts(); err != nil { + t.Fatal(err) + } + got, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(got) != "[]" { + t.Fatalf("retry wrote %q", got) + } +} diff --git a/internal/alerts/intent_checkpoint_unchanged_unix_test.go b/internal/alerts/intent_checkpoint_unchanged_unix_test.go new file mode 100644 index 0000000000..70dc73c22a --- /dev/null +++ b/internal/alerts/intent_checkpoint_unchanged_unix_test.go @@ -0,0 +1,45 @@ +//go:build !windows + +package alerts + +import ( + "os" + "path/filepath" + "testing" +) + +func TestIntentCheckpointRepairsUnsafeDestination(t *testing.T) { + for _, kind := range []string{"permissions", "symlink"} { + t.Run(kind, func(t *testing.T) { + m := &Manager{alertsDir: t.TempDir()} + save := func() { + t.Helper() + if err := m.SaveActiveAlerts(); err != nil { + t.Fatal(err) + } + } + save() + path := filepath.Join(m.alertsDir, intentPendingFileName) + if kind == "permissions" { + if err := os.Chmod(path, 0644); err != nil { + t.Fatal(err) + } + } else { + if err := os.Rename(path, path+".target"); err != nil { + t.Fatal(err) + } + if err := os.Symlink(path+".target", path); err != nil { + t.Fatal(err) + } + } + save() + info, err := os.Lstat(path) + if err != nil { + t.Fatal(err) + } + if !info.Mode().IsRegular() || info.Mode().Perm() != alertsFilePerm { + t.Fatalf("unsafe state accepted: %v", info.Mode()) + } + }) + } +} diff --git a/internal/alerts/intent_persistence.go b/internal/alerts/intent_persistence.go index 8ba0cd345d..ac40c8f681 100644 --- a/internal/alerts/intent_persistence.go +++ b/internal/alerts/intent_persistence.go @@ -1,9 +1,11 @@ package alerts import ( + "bytes" "encoding/json" "errors" "fmt" + "io" "os" "path/filepath" "sort" @@ -30,6 +32,19 @@ func (m *Manager) saveIntentPendingSnapshot() error { if err := os.MkdirAll(alertsDir, alertsDirPerm); err != nil { return fmt.Errorf("create alert intent state directory: %w", err) } + finalPath := filepath.Join(alertsDir, intentPendingFileName) + // SaveActiveAlerts serializes snapshots and writes. Check the actual bounded + // file bytes so missing, stale and failed checkpoints remain retryable. + if info, err := os.Lstat(finalPath); err == nil && info.Mode().IsRegular() && + info.Mode().Perm() == alertsFilePerm && info.Size() == int64(len(data)) { + if file, err := os.Open(finalPath); err == nil { + existing, readErr := io.ReadAll(io.LimitReader(file, int64(len(data))+1)) + closeErr := file.Close() + if readErr == nil && closeErr == nil && bytes.Equal(existing, data) { + return nil + } + } + } tmp, err := os.CreateTemp(alertsDir, "intent-pending-*.json.tmp") if err != nil { return fmt.Errorf("create alert intent state temp file: %w", err) @@ -51,7 +66,6 @@ func (m *Manager) saveIntentPendingSnapshot() error { if err := tmp.Close(); err != nil { return fmt.Errorf("close alert intent state temp file: %w", err) } - finalPath := filepath.Join(alertsDir, intentPendingFileName) if err := os.Rename(tmpName, finalPath); err != nil { return fmt.Errorf("persist alert intent pending state: %w", err) }