diff --git a/CHANGELOG.md b/CHANGELOG.md index f4d5009..8090e51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,34 @@ +## v0.216.0 — one physical disk, one verdict (2026-08-14, R-335) +**MinAgent: 0.129.0** + +**Found on live hardware within two hours of the v0.215.0 deploy, by reading the artefact the release +itself introduced.** The first two hourly checks on demo-hp logged *"3 disk(s) evaluated"* while the +persisted `disk-health-state.json` held only **two** records. The discrepancy was the bug: demo-hp's +`c11-scratch` and `felhom-backup` are the same physical NVMe (`/dev/nvme0n1`) and resolve to the same +`diskKey`, so one disk was walked twice in a single run. + +**Why that was not cosmetic.** The loop writes a disk's new record before the next entry reads it, so +the SECOND copy of an aliased disk consumed the FIRST copy's write as its prior. The disk therefore +**sustained against itself** and reached **Hiba on a FIRST sighting** — defeating truth-table row 6, +the rule the whole v0.215.0 ladder is built on and the one thing standing between a one-hour benign +excursion and a false critical alert. It would also have emitted **two identical events** for one +drive. Nothing fired on demo-hp because all three entries are healthy with zero counters, so the fault +was latent, not active — but any aliased disk developing a single pending sector would have gone +straight to Hiba. + +**Fix:** `RunDiskHealthCheck` evaluates each `diskKey` **once per run** (`internal/web/disk_health.go`). +Both entries are still marked `seen`, so neither is mistaken for a disappeared disk, and the card still +renders **both** storage rows — the dedup is about state and alerts, not display. + +**Pinned by `TestDiskCheck_SameDiskTwiceIsEvaluatedOnce`**, which asserts the first sighting stays +Figyelmeztetés and silent, that the stored prior is not this run's own write, that the second run emits +exactly ONE event, and that the card still shows two rows. Companion red-proof run and reverted: +deleting the guard makes the first sighting emit `Kind:2` (Hiba-from-sectors) at 8 sectors — the exact +false critical described above. + +This is the shape §3 of the workspace rules warns about: the release's own positive observable +("N disks evaluated") disagreed with its own persisted artefact, and only reading BOTH exposed it. + ## v0.215.0 — the disk alert that never sent (2026-08-14, R-328..R-333) **MinAgent: 0.129.0** (unchanged — every field this reads has been on the wire since agent v0.94.0/0.95.0) diff --git a/controller/internal/web/disk_health.go b/controller/internal/web/disk_health.go index 5e2bd58..5601be2 100644 --- a/controller/internal/web/disk_health.go +++ b/controller/internal/web/disk_health.go @@ -201,6 +201,21 @@ func (s *Server) RunDiskHealthCheck(ctx context.Context) error { continue } key := diskKey(d) + // ONE physical disk can appear as SEVERAL storage entries — on demo-hp the same NVMe is both + // `c11-scratch` and `felhom-backup`, and both resolve to the same durable id. They must be + // evaluated ONCE per run, for two reasons: + // + // 1. Correctness. The loop writes this run's record before the next entry reads it, so the + // second copy of the same disk would consume the FIRST copy's write as its prior — i.e. + // the disk would sustain against itself and reach Hiba on a FIRST sighting, defeating the + // entire sustain rule (truth-table row 6). + // 2. One disk, one alert. Two entries would otherwise emit two identical events. + // + // Found on live hardware after the v0.215.0 deploy: the check logged "3 disk(s) evaluated" + // while the persisted state held two records. Pinned by TestDiskCheck_SameDiskTwiceIsEvaluatedOnce. + if seen[key] { + continue + } seen[key] = true prev := s.diskHealth.records[key] prior := s.priorFor(key) diff --git a/controller/internal/web/disk_health_test.go b/controller/internal/web/disk_health_test.go index 16e7c4d..eca6e16 100644 --- a/controller/internal/web/disk_health_test.go +++ b/controller/internal/web/disk_health_test.go @@ -554,6 +554,62 @@ func TestDiskCheck_DisappearedDiskIsForgotten(t *testing.T) { } } +// ── One physical disk, several storage entries ────────────────────────────────────────────────── + +// FOUND ON LIVE HARDWARE (demo-hp, 2026-08-14): the check reported "3 disk(s) evaluated" while the +// persisted state held only TWO records, because `c11-scratch` and `felhom-backup` are the same NVMe +// and resolve to the same durable id. +// +// That aliasing was a real defect, not a cosmetic one. The loop writes this run's record before the +// next entry reads it, so the second copy of a disk consumed the FIRST copy's write as its prior — +// the disk sustained against ITSELF and reached Hiba on a first sighting, defeating truth-table row 6 +// entirely, and emitted two identical events while doing it. +// +// Red-proof: delete the `if seen[key] { continue }` guard in RunDiskHealthCheck → the first run +// reaches Fail and emits, and both assertions below fail. +func TestDiskCheck_SameDiskTwiceIsEvaluatedOnce(t *testing.T) { + h := newDiskHarness(t) + // Two storage entries, ONE physical device — identical durable id, as the agent really reports it. + twice := func() []agentapi.DiskInfo { + sm := &agentapi.SmartSummary{Health: agentapi.SmartPassed, + PendingSectors: smartPtr(8), OfflineUncorrectable: smartPtr(8), ReallocatedSectors: smartPtr(0)} + return []agentapi.DiskInfo{ + {Name: "c11-scratch", Type: "local-dir", BackingDevice: "/dev/nvme0n1", DurableID: "uuid:nvme-1", Smart: sm}, + {Name: "felhom-backup", Type: "local-dir", BackingDevice: "/dev/nvme0n1", DurableID: "uuid:nvme-1", Smart: sm}, + } + } + h.set(twice()...) + h.run(t) + + // FIRST sighting: silent, and Figyelmeztetés — a disk must not sustain against its own alias. + if n := len(h.events()); n != 0 { + t.Fatalf("first sighting of an aliased disk must be silent, got %d: %+v", n, h.events()) + } + rec := h.record(t, "uuid:nvme-1") + if rec == nil { + t.Fatal("no record for the aliased disk") + } + if agentapi.DiskVerdict(rec.Verdict) != agentapi.DiskVerdictWarn { + t.Errorf("first sighting verdict = %d, want Warn — the disk sustained against its own alias", + rec.Verdict) + } + if rec.PriorSawUncorrectable { + t.Error("the prior used on a first sighting must be empty, not this run's own write") + } + + // SECOND run: now genuinely sustained → exactly ONE event, not one per storage entry. + h.set(twice()...) + h.run(t) + if n := len(h.events()); n != 1 { + t.Errorf("one physical disk must produce ONE event, got %d: %+v", n, h.events()) + } + + // Both entries still render on the card — dedup is about state and alerts, not display. + if rows := h.s.diskHealthRows(context.Background()); len(rows) != 2 { + t.Errorf("card must still show both storage entries, got %d rows", len(rows)) + } +} + // An unreachable agent changes nothing at all — no state churn, no alarm, no error. func TestDiskCheck_UnreachableAgentIsInert(t *testing.T) { h := newDiskHarness(t)