v0.173.0 — R-77: endpoint-drift detection, samba protected-set gate, channel log honesty

Source: felhom.eu/documentation/audits/DIAG-agent-channel-2026-07-26.md bootstrap.DetectEndpointDrift names a controller.yaml vs bootstrap.json local_api.endpoint divergence -- one ERROR carrying BOTH values and BOTH paths, its own event type local_api_endpoint_drift, and its own Hungarian banner shown ABOVE the channel banner because drift is the cause and "agent unreachable" the symptom. It writes NOTHING: reconciling from bootstrap.json would clobber a correct controller.yaml on any half-provisioned or hand-repaired guest, so the authority ruling is deferred to R-78. Fail-safe silent on absent/unparseable/ incomplete bootstrap and on an empty endpoint (ensureLocalAPI's fill-if-missing path is untouched). Fingerprint compared as a BOOLEAN only; token never compared, logged or exposed. EffectiveProtected now gates samba on Enabled && UserSet, mirroring BOTH of reconcileSambaAt's early returns, and the doc comment is corrected in the same change -- it claimed "detection and deployment agree in both directions" while citing only !smb.Enabled, an assertion that went false when !smb.UserSet was added. Not over-suppressed: sharing on WITH a password and a dead container still alarms. Channel log: the debounce placeholder is stateUnconfirmed (rendered "unseeded") instead of "up", so a born-down channel no longer logs "up->down" and orUnseeded stops being dead code. Logging only -- the placeholder is still matched in the re-arm condition, so F2 born-down alerting is byte-for-byte unchanged and all nine pre-existing channelhealth tests pass. Tests 951 -> 959, all green. Red-proofs A (both directions), E and F. MinAgent unchanged; felhom-agent untouched.
2026-07-26 09:13:52 +02:00
parent c7a3a90782
commit 9056f01fae
11 changed files with 648 additions and 19 deletions
@@ -105,6 +105,11 @@ func classify(constructionErr bool, err error) classification {
 	}
 }

+// stateUnconfirmed is the debounce placeholder for a checker that has never observed a healthy
+// probe. It is deliberately NOT "up": it must not be reported as an observation. See Check's
+// debounce branch and orUnseeded.
+const stateUnconfirmed = "unconfirmed"
+
 // Checker holds the in-memory channel state. No persistence — the state is re-derived each run
 // (mirrors the AlertManager's state-based model). Safe for the single scheduler caller; the mutex
 // guards against an overlapping run.
@@ -114,7 +119,7 @@ type Checker struct {
 	logger *log.Logger

 	mu              sync.Mutex
-	state           string // "" (unseeded) | "up" | "down:<reason>"
+	state           string // "" (unseeded) | stateUnconfirmed (debounce placeholder) | "up" | "down:<reason>"
 	consecutiveDown int
 	alerted         bool // have we emitted a down alert for the CURRENT down-spell? (F2: drives
 	// alerting instead of `prev==""`, so a BORN-down — broken at startup/reseed — alerts too, not
@@ -156,10 +161,17 @@ func (c *Checker) Check(ctx context.Context) error {
 	c.consecutiveDown++
 	if cls.debounce && c.consecutiveDown < debounceThreshold {
 		// A transient blip (e.g. the ~1s agent-restart socket gap, or the agent not yet up on a cold
-		// boot). Hold the previous state — do NOT flip the dashboard or notify. Unseeded → assume up
-		// until confirmed (so a transient born-down still needs N>=2 before it alerts).
+		// boot). Hold the previous state — do NOT flip the dashboard or notify.
+		//
+		// An unseeded checker is held as "not yet confirmed down" so a transient born-down still needs
+		// N>=2 before it alerts — the debounce must apply to a cold boot exactly as it does to a live
+		// blip. R-77: that hold used to be spelled `c.state = "up"`, which made a BORN-DOWN channel log
+		// `up->down` and left orUnseeded dead code. During the 2026-07-25 outage the log therefore
+		// implied a working channel degrading, when in truth neither controller had EVER reached its
+		// agent — which actively misdirected the first read of the incident. The debounce semantics are
+		// unchanged; only the state label is honest now.
 		if c.state == "" {
-			c.state = "up"
+			c.state = stateUnconfirmed
 		}
 		c.logger.Printf("[DEBUG] [channel] transient down (%s, %d/%d) — suppressed pending confirmation: %v",
 			cls.reason, c.consecutiveDown, debounceThreshold, perr)
@@ -172,7 +184,10 @@ func (c *Checker) Check(ctx context.Context) error {
 	newState := "down:" + string(cls.reason)
 	c.sink.SetDashboard(true, cls.reason, cls.hungarian) // dashboard reflects current state always
 	prev := c.state
-	if prev == "" || prev == "up" || prev != newState {
+	// stateUnconfirmed counts as unseeded for BOTH the re-arm decision and the log label: it is the
+	// debounce placeholder, never an observed up. Keeping it in this condition preserves the F2
+	// born-down alerting behaviour byte-for-byte (it used to be spelled "up" and matched here).
+	if prev == "" || prev == "up" || prev == stateUnconfirmed || prev != newState {
 		c.alerted = false
 	}
 	c.state = newState
@@ -185,8 +200,11 @@ func (c *Checker) Check(ctx context.Context) error {
 	return nil
 }

+// orUnseeded renders a state for the log. Both the never-observed state ("") and the debounce
+// placeholder render as "unseeded" — a born-down channel must never be logged as `up->down`, which
+// is what R-77 fixed. Before that, the placeholder was literally "up" and this function was dead code.
 func orUnseeded(s string) string {
-	if s == "" {
+	if s == "" || s == stateUnconfirmed {
 		return "unseeded"
 	}
 	return s
@@ -1,10 +1,12 @@
 package channelhealth

 import (
+	"bytes"
 	"context"
 	"errors"
 	"io"
 	"log"
+	"strings"
 	"testing"
 )

@@ -272,3 +274,99 @@ func TestProbe_SeamOnly(t *testing.T) {
 		t.Fatalf("checker should call the probe seam exactly once, got %d", p.calls)
 	}
 }
+
+// R-77 Scenario F — the confirmed-down line must distinguish BORN-DOWN from a real transition.
+//
+// The debounce branch used to seed an unseeded state to "up", so a channel that had NEVER reached
+// its agent logged `up->down:unreachable` and orUnseeded was dead code. On 2026-07-25 that made the
+// log imply a working channel degrading, when in truth neither controller had ever been up — it
+// actively misdirected the first read of the incident.
+//
+// THE OTHER HALF OF THIS TEST IS THE POINT: alerting, debounce and dashboard behaviour must be
+// byte-for-byte unchanged. A logging fix that shifts alerting is a regression in a cosmetic disguise,
+// so the sink calls are asserted in COUNT and ARGUMENTS, not just the log string.
+func TestScenarioF_BornDownLogsUnseededNotUp(t *testing.T) {
+	var logbuf bytes.Buffer
+	sink := &fakeSink{}
+	c := New(nil, sink, log.New(&logbuf, "", 0))
+	// A debounced reason (connection refused → unreachable), failing from the very first probe.
+	p := &scriptedProbe{steps: []struct {
+		cons bool
+		err  error
+	}{step(false, errors.New("dial tcp 192.168.0.87:8443: connect: connection refused"))}}
+	c.probe = p.fn
+
+	// Probe 1: debounced, suppressed — no dashboard flip, no alert.
+	if err := c.Check(context.Background()); err != nil {
+		t.Fatal(err)
+	}
+	if sink.dashDown || len(sink.downs) != 0 {
+		t.Fatalf("probe 1 must be suppressed by debounce: dashDown=%v downs=%d", sink.dashDown, len(sink.downs))
+	}
+	// The placeholder must NOT be the string "up".
+	if got := c.State(); got == "up" {
+		t.Error("an unseeded checker must not report state \"up\" after a suppressed first failure")
+	}
+
+	// Probe 2: confirmed down.
+	if err := c.Check(context.Background()); err != nil {
+		t.Fatal(err)
+	}
+
+	logged := logbuf.String()
+	if !strings.Contains(logged, "unseeded->down:unreachable") {
+		t.Errorf("born-down must log \"unseeded->down:unreachable\"; got:\n%s", logged)
+	}
+	if strings.Contains(logged, "up->down") {
+		t.Errorf("a channel that was never up must NOT log \"up->down\"; got:\n%s", logged)
+	}
+
+	// --- the non-change assertions ---
+	if len(sink.downs) != 1 {
+		t.Fatalf("exactly ONE down alert must fire (F2 born-down behaviour unchanged), got %d", len(sink.downs))
+	}
+	got := sink.downs[0]
+	if got.reason != ReasonUnreachable || got.eventType != "agent_channel_unreachable" || got.severity != "warning" {
+		t.Errorf("alert arguments changed: %+v — Part 3 is a LOGGING fix only", got)
+	}
+	if !sink.dashDown || sink.dashReason != ReasonUnreachable {
+		t.Errorf("dashboard must be set down/unreachable, got down=%v reason=%q", sink.dashDown, sink.dashReason)
+	}
+	if sink.recovered != 0 {
+		t.Errorf("no recovery must fire, got %d", sink.recovered)
+	}
+	if p.calls != 2 {
+		t.Errorf("debounce threshold unchanged: expected 2 probes, got %d", p.calls)
+	}
+}
+
+// A REAL up->down transition must still log "up->down" — the fix must not relabel everything.
+func TestScenarioF_RealTransitionStillLogsUp(t *testing.T) {
+	var logbuf bytes.Buffer
+	sink := &fakeSink{}
+	c := New(nil, sink, log.New(&logbuf, "", 0))
+	p := &scriptedProbe{steps: []struct {
+		cons bool
+		err  error
+	}{
+		step(false, nil), // observed UP for real
+		step(false, errors.New("connect: connection refused")),
+		step(false, errors.New("connect: connection refused")),
+	}}
+	c.probe = p.fn
+	for i := 0; i < 3; i++ {
+		if err := c.Check(context.Background()); err != nil {
+			t.Fatal(err)
+		}
+	}
+	logged := logbuf.String()
+	if !strings.Contains(logged, "up->down:unreachable") {
+		t.Errorf("a genuine transition must still log \"up->down:unreachable\"; got:\n%s", logged)
+	}
+	if strings.Contains(logged, "unseeded->down") {
+		t.Errorf("an observed-up channel must not log \"unseeded->down\"; got:\n%s", logged)
+	}
+	if len(sink.downs) != 1 {
+		t.Errorf("exactly one down alert, got %d", len(sink.downs))
+	}
+}