From 47dda972260b4e8d257dfee97651e367fae5cd54 Mon Sep 17 00:00:00 2001 From: kami Date: Sat, 1 Aug 2026 20:43:27 +0400 Subject: [PATCH] tick: a disabled rule must not keep repeating its last alarm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `disabled_rules` stopped the loop from creating new nudges and did nothing about the ones already sent. The sev4 repeat path does not consult the rule set at all: RepeatUnacked re-sends any telegram nudge still at outcome=pending every repeat_interval (5m by default), driven by store.UnackedTelegramRules. So service_down kept arriving on a five-minute cadence after being switched off, from a row written hours earlier — two messages after the deploy, which is how it was found. That cadence, not the unsealed database, is what "she keeps spamming me" always was. The seal bug erased the acks that would have stopped it. Filter the repeat keys against the wired rule set. Filtering on wired rather than on the disabled list also silences a rule deleted from the code: nothing can ack what the UI no longer lists. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TrVSBKe3RFDF4fGYKWYQnX --- cmd/mavend/tick.go | 30 ++++++++++++++++++++++++++++++ cmd/mavend/tick_test.go | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/cmd/mavend/tick.go b/cmd/mavend/tick.go index 922e8a0..7135d56 100644 --- a/cmd/mavend/tick.go +++ b/cmd/mavend/tick.go @@ -250,6 +250,7 @@ func (t *tickLoop) tick(ctx context.Context, now time.Time) { log.Printf("tick: unacked telegram rules: %v", err) return } + keys = t.repeatableRules(keys) if len(keys) == 0 { return } @@ -261,6 +262,35 @@ func (t *tickLoop) tick(ctx context.Context, now time.Time) { } } +// repeatableRules drops keys whose rule is not wired any more. +// +// The repeat path reads the nudges table, not the rule set: any sev4 telegram +// row still at outcome=pending is re-sent every repeat_interval until it is +// acked. So turning a rule off in `disabled_rules` silenced new nudges and left +// the last un-acked one re-sending every five minutes, forever — a knob that +// stops the cause and not the symptom is worse than no knob. Found the evening +// of 2026-08-01, two messages after the rule was supposedly off. +// +// Filtering on the wired set rather than on the disabled list also covers the +// rule that was deleted from the code entirely: its orphan rows go quiet +// instead of nagging about a rule nobody can ack from the UI any more. +func (t *tickLoop) repeatableRules(keys []string) []string { + if len(keys) == 0 { + return nil + } + wired := make(map[string]bool, len(t.rules)) + for _, r := range t.rules { + wired[r.Name] = true + } + out := keys[:0:0] + for _, k := range keys { + if wired[k] { + out = append(out, k) + } + } + return out +} + // cachePhrase keeps the latest phrased nudge per rule for the sev4-repeat // path. writing under a mutex; the repeat path reads under the same. the // cache is bounded by the rule count (≤ ~30 per spec) so eviction is not a diff --git a/cmd/mavend/tick_test.go b/cmd/mavend/tick_test.go index 69dfcbd..7290ece 100644 --- a/cmd/mavend/tick_test.go +++ b/cmd/mavend/tick_test.go @@ -667,3 +667,43 @@ func TestDigestDeduplicatesByRule(t *testing.T) { t.Fatalf("after duplicate queue attempt: digestQ = %d, want 1 (dedup)", len(tl.digestQ)) } } + +// The repeat path reads the nudges table, not the rule set, so a rule turned +// off in `disabled_rules` used to keep re-sending its last un-acked telegram +// nudge every repeat_interval. Two arrived after the rule was off on +// 2026-08-01. A disabled rule must be unreachable on every path. +func TestRepeatableRulesDropsDisabledRules(t *testing.T) { + tl := &tickLoop{rules: mustRules(t, []string{"service_down"})} + got := tl.repeatableRules([]string{"service_down", "water"}) + if len(got) != 1 || got[0] != "water" { + t.Fatalf("repeatableRules = %v, want [water]", got) + } +} + +// An orphan row for a rule that no longer exists in the code goes quiet too: +// nothing can ack what the UI cannot show. +func TestRepeatableRulesDropsUnknownRules(t *testing.T) { + tl := &tickLoop{rules: loop.DefaultRules()} + if got := tl.repeatableRules([]string{"rule_deleted_last_year"}); len(got) != 0 { + t.Fatalf("repeatableRules = %v, want none", got) + } +} + +func TestRepeatableRulesKeepsWiredRules(t *testing.T) { + tl := &tickLoop{rules: loop.DefaultRules()} + got := tl.repeatableRules([]string{"service_down", "water"}) + if len(got) != 2 { + t.Fatalf("repeatableRules = %v, want both", got) + } +} + +// mustRules returns DefaultRules minus the named ones, failing if a name +// matched nothing — a typo here would make the test pass for the wrong reason. +func mustRules(t *testing.T, disabled []string) []loop.Rule { + t.Helper() + rules, dropped := loop.RulesExcept(disabled) + if len(dropped) != len(disabled) { + t.Fatalf("dropped %v, want %v", dropped, disabled) + } + return rules +}