R-116 (v0.116.0): give the backup-target flag and the gate's key the same row

The absent-drive alarm was generic while its recovery was specific -- a pair an
operator cannot match. Mechanism now measured, not reasoned (felhom.eu
audits/DIAG-r116-disks-payload-2026-07-30.md): with the device gone /disks returns
4 rows, not 3. The drive appears TWICE and the two facts the controller needs are
on different rows -- the Observe row has backup_target:true but mount_path:"" and
guest_path:"" (so driveTargetByPath registers NO key from it), while the registry
row owns /mnt/felhom-drives/<name>, the key the gate looks up, with BackupTarget
absent from its struct literal => false.

WHY v0.115.0 WAS INERT: its fallback computed StablePathForRaw(t.MountPath), and in
the absent state MountPath is ALSO "" -- emptied by the same exactMount failure
that empties BackingDevice. It assigned nothing. Its test passed because the fixture
supplied a MountPath production never supplies, and the harness left DriveTargets
nil so the union loop never ran. Both corrected here; red-proof 1 replays v0.115.0's
exact code against the real shape and it fails.

THE JOIN, which was the hard part: with the device gone the two records share no
runtime field -- no mount, no backing device, and the Observe row's DurableID has
degraded off the fs-UUID. They share CONFIGURATION: storage.cfg's path on one side,
the .mount unit's Where on the other, both yielding the same stable guest path. New
hub.StorageTarget.ConfigPath (json:"-" -- that struct is a cross-repo contract
pinned by the golden + contract_test key-set comparison, and nothing off-box needs
the value), set from s.Path in observe.go, consulted in disks.go only after MountPath
so the present-state path is byte-identical, plus a guest-path arm on the union dedup
so exactly one row carries the drive.

WHY NEITHER OBVIOUS OPTION WAS TAKEN -- both regress R-114, which shipped yesterday.
backup_target_offer.go:79 reads (BackupTarget && MountPath != "") as "a real
drive with its own mountpoint -- healthy" and returns before its TargetAbsent
branch. Back-filling MountPath onto the Observe row (the smallest change, and the
spec's lean) and teaching the registry row the flag (its MountPath is non-empty, read
from the stale unit file) BOTH manufacture that row while the drive is missing, which
would have told the customer the backup target is fine while its drive is gone.
R-114's correctness rests on the absent-state rows not combining the flag with a
mount path; that coupling was invisible until the payload existed. Pinned by
TestAbsentTargetKeepsR114DegradedSignal.

Role unchanged, BoundUnderParent conjunction not widened, no wire field changed.
Suppressing the registry row in the absent state also removes its false
state:"attached" and its root-filesystem-derived total_bytes -- R-118's symptom
goes incidentally; R-118 is NOT fixed and stays open.

Tests 845 -> 849, suite rc=0 read separately from this commit. Four red-proofs, each
mutation asserted to have landed first.

NOT live-validated at this commit: publish+vouch, C5, discrimination, over-correction.
This commit is contained in:
2026-07-30 08:48:05 +02:00
parent 2f4ccab166
commit 21b0164fad
6 changed files with 389 additions and 92 deletions
+10
View File
@@ -280,6 +280,16 @@ type StorageTarget struct {
MountPath string `json:"mount_path"` // host mountpoint (dir/usb); "" for network/lvm
BackingDevice string `json:"backing_device"` // resolved block device (e.g. /dev/sdb1); "" for network
// ConfigPath is the storage's CONFIGURED path from storage.cfg (proxmox.Storage.Path) — not a
// resolved mount. It is the only identity a dir storage keeps when its device is gone: MountPath
// and BackingDevice both empty out (observe.go's exactMount block) and DurableID degrades off the
// fs-UUID, so the configured path is what still says WHICH drive this row is about (R-116).
//
// `json:"-"` DELIBERATELY. This struct is a cross-repo contract duplicated in felhom.eu/hub and
// pinned by testdata/host-report.golden.json + contract_test.go's key-set comparison; a wire-visible
// field here would need a matching change in the other repo to stay non-drifting. Nothing off-box
// needs this value — its only consumer is the agent's own /disks construction, in-process.
ConfigPath string `json:"-"`
// ClassHint is a fast|slow HINT derived from the backing disk's rotational flag — a
// hint only; the authoritative class is hub-owned (locked decision). "" when not
// derivable (network targets have no local rotational flag).
+44 -1
View File
@@ -255,8 +255,24 @@ func (s *Server) handleDisks(w http.ResponseWriter, r *http.Request, vmid int) {
// inside the two guest-path blocks a system-role row never enters, so it stays false, and
// planDriveGates computes present[gp] = present[gp] || d.BoundUnderParent. Inert by construction
// — pinned by TestAbsentTargetRowDoesNotRegisterPresence.
//
// v0.116.0 — WHY v0.115.0 (the MountPath-only form) WAS INERT, measured not reasoned. In the
// absent state t.MountPath is ALSO "" — the same exactMount failure that emptied BackingDevice
// empties it — so StablePathForRaw("") returned "" and this assigned nothing. Captured payload:
// felhom.eu audits/DIAG-r116-disks-payload-2026-07-30.md §6.2.
//
// t.ConfigPath is the fix: the storage's CONFIGURED path from storage.cfg, which is configuration
// and therefore survives the device. MountPath is tried FIRST so the present-state path and
// v0.115.0's tested behaviour are byte-identical; ConfigPath is consulted only when the mount is
// genuinely gone. MountPath is deliberately NOT back-filled from ConfigPath — see the union-dedup
// note below for the consumer that would break, and because a path that is not mounted is not a
// "host mountpoint" (this field's own contract, :152-153).
if di.GuestPath == "" && di.BackupTarget && t.BackingDevice == "" {
di.GuestPath = StablePathForRaw(t.MountPath)
if gp := StablePathForRaw(t.MountPath); gp != "" {
di.GuestPath = gp
} else {
di.GuestPath = StablePathForRaw(t.ConfigPath)
}
}
// Inspect the backing device for the UI's data-bearing hint (the authoritative check
// is re-run at format time on the actual device).
@@ -288,16 +304,43 @@ func (s *Server) handleDisks(w http.ResponseWriter, r *http.Request, vmid int) {
// NOT duplicated, and no Observe row is dropped (so this can never regress the current view).
if s.driveTargets != nil {
seen := make(map[string]bool, len(out))
// R-116: dedup ALSO by guest path. `seen` keys on MountPath, the one field the absent state
// empties, so with the device gone /mnt/<name> is absent from `seen` and the registry row was NOT
// skipped — /disks carried the drive TWICE, the Observe row holding BackupTarget with no key and
// the registry row holding both keys with BackupTarget defaulted false. driveTargetByPath
// (controller intermediary.go:602-618) assigns rather than ORs, and the registry row is appended
// LAST, so its false won on both keys. Measured, 4 rows vs 3:
// felhom.eu audits/DIAG-r116-disks-payload-2026-07-30.md §6.2.
//
// THE JOIN, and it is the whole point: with the device gone the two records of one drive share NO
// runtime field — no mount, no backing device, and the Observe row's DurableID has degraded off the
// fs-UUID. What they DO share is CONFIGURATION: the Observe row's storage path (storage.cfg) and the
// registry row's unit `Where` (the .mount unit) are the same path, so both derive the same stable
// guest path. That is the key both sides can still compute, which is why the dedup keys on it.
seenGuest := make(map[string]bool, len(out))
for _, d := range out {
if d.MountPath != "" {
seen[d.MountPath] = true
}
if d.GuestPath != "" {
seenGuest[d.GuestPath] = true
}
}
if drives, derr := s.driveTargets.Known(r.Context()); derr == nil {
for _, d := range drives {
if d.MountPath == "" || seen[d.MountPath] {
continue
}
// Same drive as an Observe row that already carries this guest path — skip it. Suppressing
// it rather than teaching it BackupTarget is deliberate: the registry row has a non-empty
// MountPath (from the unit file, stale by then), and the controller reads
// `d.BackupTarget && d.MountPath != ""` as "a real drive with its own mountpoint — HEALTHY"
// (backup_target_offer.go:79). Putting the flag on a row with a stale MountPath would have
// silently regressed R-114, telling the customer the backup target is fine while its drive
// is missing. Pinned by TestAbsentTargetKeepsR114DegradedSignal.
if gp := StablePathForRaw(d.MountPath); gp != "" && seenGuest[gp] {
continue
}
di := DiskInfo{
Name: d.Name, Type: d.Type, State: "attached",
MountPath: d.MountPath, DurableID: d.DurableID,
+159 -11
View File
@@ -32,9 +32,23 @@ import (
// under test here.
func targetRowServer(t *testing.T, primaryTarget string, targets []hub.StorageTarget) *Server {
t.Helper()
return targetRowServerWithDrives(t, primaryTarget, targets, nil)
}
// targetRowServerWithDrives additionally wires the REGISTRY union source. v0.115.0's tests left
// DriveTargets nil, so the union loop never ran and the two-row absent shape — the actual defect — was
// invisible to the whole suite. Any test about which row carries what MUST populate this.
func targetRowServerWithDrives(t *testing.T, primaryTarget string, targets []hub.StorageTarget,
drives []storage.KnownTarget) *Server {
t.Helper()
var known storage.KnownTargets
if drives != nil {
known = fakeKnownTargets{drives: drives}
}
srv, err := NewServer(Options{
ListenAddr: "127.0.0.1:0",
Guests: &fakeGuestsCfg{}, Backups: &fakeBackups{}, Store: &fakeStore{},
DriveTargets: known,
ListenAddr: "127.0.0.1:0",
Guests: &fakeGuestsCfg{}, Backups: &fakeBackups{}, Store: &fakeStore{},
Storage: fakeStorage{targets: targets},
// Service is REQUIRED: normalizeBackupTiers (backup_tiers.go:21-22) drops any tier with a nil
// Service, and the legacy fallback then yields TargetID "" — which silently makes every
@@ -86,11 +100,32 @@ func isTargetByPath(disks []map[string]any) map[string]bool {
return out
}
// theAbsentTarget is the Session-C shape: the felhom-backup storage whose device has gone, so Observe
// reports no backing device — which is what flips its role to system and drops its guest path.
// theAbsentTarget is the absent-target Observe row, CORRECTED in v0.116.0 to the shape the live box
// actually produces.
//
// THIS FIXTURE IS WHY AN INERT FIX SHIPPED GREEN. As written for v0.115.0 it supplied
// `MountPath: "/mnt/mentes"` — a field the real absent state does NOT have. The same exactMount failure
// that empties BackingDevice empties MountPath (observe.go:184-190), so on the live box this row carries
// `mount_path: ""`, and v0.115.0's `StablePathForRaw(t.MountPath)` was therefore
// `StablePathForRaw("")` == "". The fixture handed the code a value production never supplies, the test
// went green, and the fix was inert on real hardware — twice.
//
// Captured payload this now mirrors, field for field:
// felhom.eu audits/DIAG-r116-disks-payload-2026-07-30.md §6.2.
var theAbsentTarget = hub.StorageTarget{
Name: "felhom-backup", Type: hub.StorageTypeLocalDir,
MountPath: "/mnt/mentes", BackingDevice: "", State: hub.StorageStateDisconnected,
MountPath: "", BackingDevice: "", ConfigPath: "/mnt/mentes",
State: hub.StorageStateDisconnected,
// DurableID degrades off the fs-UUID exactly as the live payload showed (`path:/mnt/cel` there).
DurableID: "path:/mnt/mentes",
}
// theAbsentRegistryRow is the OTHER half of the live absent payload — the registry/union row. Its
// MountPath comes from the systemd .mount unit FILE (registry_known.go:40-75), which never consults the
// mount table, so it survives the device intact. Its presence is what made /disks carry the drive TWICE.
var theAbsentRegistryRow = []storage.KnownTarget{
{Name: "9303-uuid", Type: hub.StorageTypeUSB, MountPath: "/mnt/mentes",
DurableID: "uuid:9303", UUID: "9303"},
}
// ── the observable that must move ───────────────────────────────────────────────────────────────
@@ -98,7 +133,8 @@ var theAbsentTarget = hub.StorageTarget{
// RED-PROOF: delete the `di.GuestPath == "" && di.BackupTarget && t.BackingDevice == ""` block and
// this fails with "the guest path the controller keys on is MISSING from /disks entirely".
func TestAbsentBackupTargetIsResolvableByGuestPath(t *testing.T) {
disks := wireDisks(t, targetRowServer(t, "felhom-backup", []hub.StorageTarget{theAbsentTarget}))
disks := wireDisks(t, targetRowServerWithDrives(t, "felhom-backup",
[]hub.StorageTarget{theAbsentTarget}, theAbsentRegistryRow))
isTarget := isTargetByPath(disks)
const guestPath = "/mnt/felhom-drives/mentes"
@@ -109,11 +145,9 @@ func TestAbsentBackupTargetIsResolvableByGuestPath(t *testing.T) {
"can never fire (R-116)", guestPath)
}
if !got {
t.Errorf("isTarget[%q] = false; the row carrying the guest path does not carry the flag", guestPath)
}
// The host-path key was never the broken one — it must stay true.
if !isTarget["/mnt/mentes"] {
t.Error("isTarget by host path regressed to false")
t.Fatalf("isTarget[%q] = FALSE. Both rows for this drive reached the wire and the registry row — "+
"appended last, BackupTarget defaulted false — overwrote the flag-bearing row's true. This is "+
"the measured live defect, not a hypothetical: rows=%d", guestPath, len(disks))
}
}
@@ -192,3 +226,117 @@ func TestNonTargetDriveNeverCarriesTheFlag(t *testing.T) {
}
}
}
// ── v0.116.0 — the join, and the regression it must not cause ───────────────────────────────────
// THE JOIN. With the device gone the two records of one drive share no runtime field, so the dedup has
// to key on the one thing both can still derive: the CONFIGURED path (storage.cfg's `path` on the
// Observe side, the .mount unit's `Where` on the registry side), expressed as the stable guest path.
// This pins that exactly one row survives — because driveTargetByPath ASSIGNS rather than ORs, so two
// rows disagreeing on the flag is decided by append order, which is not a contract anyone should rely on.
//
// RED-PROOF: delete the `seenGuest[gp]` skip in the union loop and this fails with rows=2.
func TestAbsentTargetAppearsExactlyOnce(t *testing.T) {
disks := wireDisks(t, targetRowServerWithDrives(t, "felhom-backup",
[]hub.StorageTarget{theAbsentTarget}, theAbsentRegistryRow))
const guestPath = "/mnt/felhom-drives/mentes"
var rows []map[string]any
for _, d := range disks {
if gp, _ := d["guest_path"].(string); gp == guestPath {
rows = append(rows, d)
}
}
if len(rows) != 1 {
t.Fatalf("the absent drive is carried by %d rows, want exactly 1 — with two rows the flag the "+
"controller reads is decided by append order, not by the fix. rows=%v", len(rows), rows)
}
if bt, _ := rows[0]["backup_target"].(bool); !bt {
t.Error("the surviving row does not carry backup_target=true")
}
}
// THE REGRESSION THIS FIX MUST NOT CAUSE, and the reason neither obvious option was taken.
//
// The controller reads `d.BackupTarget && d.MountPath != ""` as "a real drive with its own mountpoint —
// HEALTHY" and returns immediately (backup_target_offer.go:79). So the two candidate fixes that look
// smallest — back-filling MountPath onto the Observe row, or teaching the registry row the flag (its
// MountPath is non-empty, read from the stale unit file) — BOTH produce a row satisfying that predicate
// while the drive is missing. Either would have silently regressed R-114, which shipped 2026-07-29 and
// tells the customer „A rendszermentés meghajtója nem érhető el" in exactly this state, flipping it back
// to a false healthy.
//
// R-114's correctness currently rests on the absent-state rows NOT combining the flag with a mount path.
// That coupling was invisible until the payload was captured, and it is what this test pins.
//
// RED-PROOF: set `MountPath: "/mnt/mentes"` on theAbsentTarget (v0.115.0's fixture value) and this fails.
func TestAbsentTargetKeepsR114DegradedSignal(t *testing.T) {
disks := wireDisks(t, targetRowServerWithDrives(t, "felhom-backup",
[]hub.StorageTarget{theAbsentTarget}, theAbsentRegistryRow))
for _, d := range disks {
bt, _ := d["backup_target"].(bool)
mp, _ := d["mount_path"].(string)
if bt && mp != "" {
t.Fatalf("row %v carries backup_target=true AND mount_path=%q while the drive is ABSENT. "+
"resolveBackupTargetState (backup_target_offer.go:79) reads that as \"a real drive with "+
"its own mountpoint — healthy\" and returns before its TargetAbsent branch, so the "+
"customer is told the backup target is fine while its drive is gone. That is R-114, "+
"regressed.", d["name"], mp)
}
}
}
// PRESENT-STATE PARITY. The fix must change nothing when the drive is there. Present state is the
// state every healthy box is in, so a change here reaches the whole fleet; absent state reaches only a
// box with a problem. Both rows are supplied, exactly as on a live present box, and the pre-existing
// MountPath dedup must still collapse them to one COMPLETE row.
func TestPresentTargetPayloadUnchanged(t *testing.T) {
present := hub.StorageTarget{
Name: "felhom-backup", Type: hub.StorageTypeLocalDir,
MountPath: "/mnt/mentes", BackingDevice: "/dev/sdb", ConfigPath: "/mnt/mentes",
State: hub.StorageStateAttached, DurableID: "uuid:9303",
}
disks := wireDisks(t, targetRowServerWithDrives(t, "felhom-backup",
[]hub.StorageTarget{present}, theAbsentRegistryRow))
var rows []map[string]any
for _, d := range disks {
if d["name"] == "felhom-backup" || d["mount_path"] == "/mnt/mentes" {
rows = append(rows, d)
}
}
if len(rows) != 1 {
t.Fatalf("present state carries the drive on %d rows, want 1 (the MountPath dedup): %v", len(rows), rows)
}
r := rows[0]
for field, want := range map[string]any{
"mount_path": "/mnt/mentes", "guest_path": "/mnt/felhom-drives/mentes",
"backing_device": "/dev/sdb", "role": "user-data", "state": "attached",
"backup_target": true, "bound_under_parent": true, "durable_id": "uuid:9303",
} {
if got := r[field]; got != want {
t.Errorf("present-state %s = %v, want %v — the fix altered the healthy payload", field, got, want)
}
}
}
// The negative, with the union loop actually running: a non-target absent drive gains the flag on no row
// and keeps its own registry row (nothing to dedup against, since no Observe row claims its guest path).
func TestAbsentNonTargetKeepsItsRegistryRowAndNoFlag(t *testing.T) {
disks := wireDisks(t, targetRowServerWithDrives(t, "felhom-backup",
[]hub.StorageTarget{{Name: "adat", Type: hub.StorageTypeLocalDir, MountPath: "",
BackingDevice: "", ConfigPath: "/mnt/adat", State: hub.StorageStateDisconnected}},
[]storage.KnownTarget{{Name: "adat-uuid", Type: hub.StorageTypeUSB,
MountPath: "/mnt/adat", DurableID: "uuid:1111", UUID: "1111"}}))
isTarget := isTargetByPath(disks)
for k, v := range isTarget {
if v {
t.Errorf("isTarget[%q] = true for a NON-target drive — the BackupTarget gate failed", k)
}
}
if _, ok := isTarget["/mnt/felhom-drives/adat"]; !ok {
t.Error("the non-target drive lost its guest-path key entirely — the union row was over-suppressed")
}
}
+10 -3
View File
@@ -235,9 +235,16 @@ func (o *Observer) build(s proxmox.Storage, mounts []Mount) observed {
Content: s.Content,
MountPath: mountPath,
BackingDevice: backingDevice,
ClassHint: classHint,
Role: "", // hub-owned; not derivable from a Proxmox def (slice 10)
Smart: hub.SmartSummary{Health: hub.SmartUnknown},
// R-116: the CONFIGURED path, carried verbatim and never resolved. This is emphatically NOT the
// fallthrough the comment above forbids — that prohibition is about resolving a device or a UUID
// from the CONTAINING filesystem when the target is not its own mount, which would hand back
// root's identity and mis-target a DR re-attach. `s.Path` is the storage's own declaration of
// where it lives; it identifies nothing but itself, and it is not used for device or UUID
// resolution anywhere. MountPath stays empty when the mount is gone, which is the truth.
ConfigPath: s.Path,
ClassHint: classHint,
Role: "", // hub-owned; not derivable from a Proxmox def (slice 10)
Smart: hub.SmartSummary{Health: hub.SmartUnknown},
}
// Thin-pool DATA fill: surfaced prominently for lvmthin (metadata fill is Phase B/lvs).