Judge S21 by time since the first failing report while the machine is heard, not by how many reports said it (issue 233 review)

cmd/mesh-controller/apply_failing_test.go

@@ -1,10 +1,12 @@
 package main
 
 import (
+	"context"
 	"strings"
 	"testing"
 	"time"
 
+	"github.com/novox/mesh-controller/internal/conditions"
 	"github.com/novox/mesh-controller/internal/inventory"
 )
 
@@ -37,8 +39,8 @@ func TestAPackageManagerThatDoesNotAnswerIsRaisedNamingItsWords(t *testing.T) {
 	if o.Key() != "machine.anchor.apply-failing" || o.Machine != "anchor" {
 		t.Errorf("the condition is not the machine's: %s", o.Key())
 	}
-	if !strings.Contains(o.Summary, "pacman does not answer") || !strings.Contains(o.Summary, "18 reports") {
-		t.Errorf("the summary does not name the tool and how long: %s", o.Summary)
+	if !strings.Contains(o.Summary, "pacman does not answer") || !strings.Contains(o.Summary, "since 2026-10-04 12:30") {
+		t.Errorf("the summary does not name the tool and since when: %s", o.Summary)
 	}
 	if strings.Contains(o.Summary, "/etc/") {
 		t.Errorf("the summary carries a path, which belongs in the evidence: %s", o.Summary)
@@ -66,18 +68,56 @@ func TestARefusalThatGoesOnIsRaised(t *testing.T) {
 	}
 }
 
-// **One attempt is not a fault that goes on, and a silent or sleeping machine is S1's**: a laptop shut for a
-// week comes back with one report of an old failure; a machine not heard from says silent, not this.
-func TestOneReportOrAQuietMachineIsNotRaised(t *testing.T) {
+// **Judged by time while the machine is heard, not by how many reports said it** (review of novox/hq issue 233):
+// a node-engine says the same failure once, so one report an apply interval old is a fault that goes on; one
+// younger is not yet; and a silent or sleeping machine is S1's.
+func TestOneReportOlderThanAnApplyIsRaisedAndAQuietMachineIsNot(t *testing.T) {
 	now := time.Date(2026, 10, 4, 14, 0, 0, 0, time.UTC)
 	f := calm(now)
-	f.doing = []inventory.Doing{unansweringPackageManager(now, 7*24*time.Hour, 1)}
-	if got := watchApplyFailing(f); len(got) != 0 {
-		t.Errorf("one report was raised as a failure that goes on: %+v", got)
+	f.doing = []inventory.Doing{unansweringPackageManager(now, 6*time.Minute, 1)}
+	if got := watchApplyFailing(f); len(got) != 1 {
+		t.Errorf("one report of a failure six minutes old was not raised: %+v", got)
 	}
-	f.doing = []inventory.Doing{unansweringPackageManager(now, time.Hour, 12)}
+	f.doing = []inventory.Doing{unansweringPackageManager(now, 4*time.Minute, 1)}
+	if got := watchApplyFailing(f); len(got) != 0 {
+		t.Errorf("a failure four minutes old was raised before one apply could clear it: %+v", got)
+	}
+	f.doing = []inventory.Doing{unansweringPackageManager(now, time.Hour, 1)}
 	f.machines[0].lastHeard = now.Add(-time.Hour)
 	if got := watchApplyFailing(f); len(got) != 0 {
 		t.Errorf("a silent machine was raised for its last report: %+v", got)
 	}
+	f = calm(now)
+	f.doing = []inventory.Doing{unansweringPackageManager(now, time.Hour, 1)}
+	f.doing[0].Node = "a-machine-the-mesh-does-not-know"
+	if got := watchApplyFailing(f); len(got) != 0 {
+		t.Errorf("a machine not heard of was raised: %+v", got)
+	}
+}
+
+// **A failed re-apply followed by a clean one clears S21** (review of novox/hq issue 233): the node-engine says
+// the clean re-apply, the store keeps it as applied with no failure since, and the condition the failure raised
+// is cleared through the keeper on the next tick.
+func TestACleanReapplyAfterAFailedOneClearsTheCondition(t *testing.T) {
+	now := time.Date(2026, 10, 4, 14, 0, 0, 0, time.UTC)
+	store := conditions.NewInMemory()
+	k := conditions.NewKeeper(t.Context(), conditions.Options{Store: store, History: store,
+		Teller: &conditions.Told{}, Now: func() time.Time { return now }})
+	defer k.Close(context.Background())
+	w := &watchdogs{keeper: k, started: now.Add(-time.Hour)}
+
+	failing := calm(now)
+	failing.doing = []inventory.Doing{unansweringPackageManager(now, 20*time.Minute, 1)}
+	w.see(t.Context(), failing)
+	open, err := k.Open(t.Context())
+	if err != nil || len(open) != 1 || open[0].Kind != kindApplyFailing {
+		t.Fatalf("the failed re-apply raised %+v (%v), want apply-failing", open, err)
+	}
+
+	clean := calm(now.Add(5 * time.Minute))
+	clean.doing = nil // the clean re-apply recorded: NotDoingWhatTheyWereTold no longer lists the machine
+	w.see(t.Context(), clean)
+	if open, _ := k.Open(t.Context()); len(open) != 0 {
+		t.Fatalf("the condition stayed open after the clean re-apply: %+v", open)
+	}
 }