R-1124 + R-1012: forget a decommissioned drive that is gone; name an orphan thin device in a backup failure

R-1124: ForgetRetiredDrives (start-up + the 20 s tick) disables and removes the host mount unit of a
DECOMMISSIONED drive whose disk is absent and unmounted; the agent then no longer lists it. A present
decommissioned drive keeps its unit (one-click re-enroll). The file goes only through the new
felhom-priv-apply verb `unit-forget`, which re-checks marker, disabled, unmounted and absent as root.

R-1012: the read-only root helper felhom-thin-check <vg> <pool> (bundle) lists the pool's thin ids
(metadata snapshot reserved and released) next to LVM's before and after. BackupRunner.SetThinCheck runs
it before a snapshot-mode backup (log) and again after a failure, appending the finding in plain words to
the error the controller mails to the operator. Two reads, intersected; fails open; never removes.

Red-proofs: felhom.eu audits/agent-allowlist-rotation-2026-10-17/P1/. Code only; the CHANGELOG entry
follows the release.

Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS

internal/backup/r1012_thincheck_test.go

@@ -0,0 +1,64 @@
+package backup
+
+import (
+	"context"
+	"errors"
+	"strings"
+	"testing"
+
+	"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
+)
+
+// R-1012: a snapshot backup that fails carries the orphan finding in its error — the text the controller mails to
+// the operator (`whole_guest_backup_failed`). The check runs once before the run and once, FRESH, after the failure.
+// RED-PROOF (P1/redproof-thincheck.txt): with explainFailure's append removed this test fails.
+func TestBackup_SnapshotFailure_NamesTheOrphan(t *testing.T) {
+	const why = "probable cause: thin pool pve/data holds device 4 that LVM does not know"
+	for name, api := range map[string]*fakeBackupAPI{
+		"task fails":  {vzdumpUPID: "UPID:x", waitErr: errors.New("task failed: job errors")},
+		"start fails": {vzdumpErr: errors.New("vzdump: 500")},
+	} {
+		t.Run(name, func(t *testing.T) {
+			r := NewBackupRunner(api, "local", "", "", "", quiet())
+			calls := 0
+			r.SetThinCheck(func(context.Context, int) string { calls++; return why })
+			rec, err := r.Backup(context.Background(), 9201)
+			if err == nil || rec.Success {
+				t.Fatalf("want a failure, got %+v", rec)
+			}
+			if !strings.Contains(rec.Error, why) {
+				t.Fatalf("the failure does not name the orphan: %q", rec.Error)
+			}
+			if calls != 2 {
+				t.Fatalf("thin check ran %d times, want 2 (before the run and after the failure)", calls)
+			}
+		})
+	}
+}
+
+// A successful backup keeps a clean record (the pre-check only logs); a stop-mode backup takes no snapshot, so the
+// check never runs; an unwired runner behaves exactly as before.
+func TestBackup_ThinCheck_SilentWhenNotApplicable(t *testing.T) {
+	ok := &fakeBackupAPI{vzdumpUPID: "UPID:x",
+		content: []proxmox.StorageContent{{VolID: "local:backup/vzdump-lxc-9201-x", Content: "backup", VMID: 9201, Size: 100, CTime: 1}}}
+	r := NewBackupRunner(ok, "local", "", "", "", quiet())
+	calls := 0
+	r.SetThinCheck(func(context.Context, int) string { calls++; return "probable cause: x" })
+	rec, err := r.Backup(context.Background(), 9201)
+	if err != nil || !rec.Success || rec.Error != "" || calls != 1 {
+		t.Fatalf("success: err=%v rec=%+v calls=%d (want one pre-check, clean record)", err, rec, calls)
+	}
+
+	stop := NewBackupRunner(&fakeBackupAPI{vzdumpUPID: "UPID:x", waitErr: errors.New("boom")}, "local", proxmox.ModeStop, "", "", quiet())
+	calls = 0
+	stop.SetThinCheck(func(context.Context, int) string { calls++; return "probable cause: x" })
+	rec, _ = stop.Backup(context.Background(), 9201)
+	if calls != 0 || strings.Contains(rec.Error, "probable cause") {
+		t.Fatalf("stop mode: calls=%d error=%q", calls, rec.Error)
+	}
+
+	plain := NewBackupRunner(&fakeBackupAPI{vzdumpUPID: "UPID:x", waitErr: errors.New("boom")}, "local", "", "", "", quiet())
+	if rec, _ := plain.Backup(context.Background(), 9201); rec.Error != "boom" {
+		t.Fatalf("unwired: error = %q, want the task error unchanged", rec.Error)
+	}
+}

internal/backup/runner.go

@@ -61,8 +61,12 @@ type BackupRunner struct {
 	// beforeBackup runs just before the space preflight (R-1006): it removes a dead half-written vzdump of this guest on
 	// this target, so a leftover from a power cut cannot hold the space this backup needs. nil = nothing.
 	beforeBackup func(ctx context.Context, storage string, vmid int)
-	logger       *slog.Logger
-	now          func() time.Time
+	// thinCheck (R-1012) names an orphan thin device in the guest's pools in plain words ("" = none / not checked). It
+	// runs before a snapshot-mode backup (a WARN in the log) and again when the vzdump fails, so the failure the
+	// controller mails to the operator says why. It never removes anything. nil = not wired.
+	thinCheck func(ctx context.Context, vmid int) string
+	logger    *slog.Logger
+	now       func() time.Time
 	// rejected remembers the volids already announced by warnRejectedArchiveOnce, so an incomplete
 	// archive is reported ONCE rather than on every 5-minute due-check. Bounded in practice: one
 	// entry per aborted upload, and a process restart clears it. Guarded by rejectedMu because the
@@ -110,6 +114,22 @@ func (r *BackupRunner) SetBeforeBackup(fn func(ctx context.Context, storage stri
 	r.beforeBackup = fn
 }
 
+// SetThinCheck wires the R-1012 orphan check (see BackupRunner.thinCheck).
+func (r *BackupRunner) SetThinCheck(fn func(ctx context.Context, vmid int) string) { r.thinCheck = fn }
+
+// explainFailure appends the orphan finding to a failed snapshot-mode backup's error. A FRESH check (not the one from
+// before the run): the failure itself may be what made the orphan. Bounded, so a stuck helper cannot hold the job.
+func (r *BackupRunner) explainFailure(ctx context.Context, vmid int, rec *hub.Backup) {
+	if r.thinCheck == nil || r.mode != proxmox.ModeSnapshot {
+		return
+	}
+	cctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 2*time.Minute)
+	defer cancel()
+	if why := r.thinCheck(cctx, vmid); why != "" {
+		rec.Error = rec.Error + " — " + why
+	}
+}
+
 // localPruneSpec returns the `--prune-backups` spec to apply to THIS backup, or "" to skip pruning. It
 // applies the configured retention ONLY when the target is a non-PBS storage — PBS offsite retention is a
 // separate lifecycle and must never be pruned by the per-run flag (§9). Fail-safe: if the target's type
@@ -201,12 +221,18 @@ func (r *BackupRunner) backup(ctx context.Context, vmid int, onSnapshot func())
 		return rec, fmt.Errorf("backup: %s", rec.Error)
 	}
 
+	if r.thinCheck != nil && r.mode == proxmox.ModeSnapshot {
+		if why := r.thinCheck(ctx, vmid); why != "" {
+			r.logger.Warn("backup: an orphan thin device may make this snapshot backup fail (R-1012)", "vmid", vmid, "finding", why)
+		}
+	}
 	upid, err := r.api.Vzdump(ctx, proxmox.VzdumpOptions{
 		VMID: vmid, Storage: r.target, Mode: r.mode, Notes: r.notes,
 		PruneBackups: r.localPruneSpec(ctx), // local target → keep-last=N; PBS/unknown → "" (no prune)
 	})
 	if err != nil {
 		rec.Error = err.Error()
+		r.explainFailure(ctx, vmid, &rec)
 		rec.DurationSeconds = time.Since(start).Seconds()
 		return rec, fmt.Errorf("backup: vzdump vmid %d: %w", vmid, err)
 	}
@@ -220,6 +246,7 @@ func (r *BackupRunner) backup(ctx context.Context, vmid int, onSnapshot func())
 		}
 		if _, err := r.api.WaitTask(ctx, upid, proxmox.WaitOptions{Timeout: r.waitTimeout}); err != nil {
 			rec.Error = err.Error()
+			r.explainFailure(ctx, vmid, &rec)
 			rec.DurationSeconds = time.Since(start).Seconds()
 			return rec, fmt.Errorf("backup: vzdump task vmid %d: %w", vmid, err)
 		}

internal/capability/manifest.go

@@ -91,9 +91,11 @@ var manifest = []Capability{
 	{"disk-mkfs-xfs", "guarded format (xfs)", "/usr/local/sbin/felhom-mkfs-guarded", []string{"/dev/sda", "xfs"}, false, ""},
 	{"disk-smart", "disk SMART health read", "/usr/sbin/smartctl", []string{"-a", "-j", "/dev/sda"}, false, ""},
 	{"disk-lvs", "thin-pool usage read", "/usr/sbin/lvs", []string{"--reportformat", "json", "--units", "b", "-o", "lv_name,data_percent,metadata_percent", "--", "pve/data"}, false, ""},
+	{"thin-orphan-check", "orphan thin device check around a whole-guest backup (read-only, R-1012)", "/usr/local/sbin/felhom-thin-check", []string{"pve", "data"}, false, ""},
 
 	// ---- Storage mount units (watchdog re-mount) ----
 	{"mount-unit-install", "fs-UUID mount unit install (root content check, R-861)", "/usr/local/sbin/felhom-priv-apply", []string{"unit", "mnt-felhom\\x2dx.mount"}, false, ""},
+	{"mount-unit-forget", "decommissioned-drive unit removal (root check, R-1124)", "/usr/local/sbin/felhom-priv-apply", []string{"unit-forget", "mnt-felhom\\x2dx.mount"}, false, ""},
 	{"mount-daemon-reload", "systemd reload after unit write", "/usr/bin/systemctl", []string{"daemon-reload"}, false, ""},
 	{"mount-unit-enable", "mount unit enable", "/usr/bin/systemctl", []string{"enable", "--now", "--", "mnt-felhom\\x2dx.mount"}, false, ""},
 	{"mount-unit-disable", "mount unit disable", "/usr/bin/systemctl", []string{"disable", "--", "mnt-felhom\\x2dx.mount"}, false, ""},

internal/storage/forget_retired.go

@@ -0,0 +1,85 @@
+package storage
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"strings"
+)
+
+// ForgetRetiredDrives (R-1124) removes the host mount unit of every DECOMMISSIONED drive that is physically gone, so a
+// later host boot no longer waits for it and the agent stops listing it (RegistryKnownTargets and
+// ReassertEnrolledMounts both enumerate the units — no unit, no entry, no „absent by UUID" line every 20 s).
+//
+// Only a drive that is BOTH decommissioned (the operator's or the customer's permanent removal) AND absent (its
+// /dev/disk/by-uuid link does not resolve) AND not mounted is forgotten. A decommissioned drive that is still plugged in
+// keeps its unit and its raw host mount — that is what makes the one-click re-enroll (H3) work, and decommission stays a
+// logical retire (`03` §storage). The intent record stays `decommissioned`: a forgotten drive that comes back is a new
+// candidate, never auto-mounted. The root half (`felhom-priv-apply unit-forget`) re-checks every one of these conditions
+// and refuses otherwise. Called at agent start, on the 20-second storage tick, and right after a decommission. Best
+// effort per unit; returns how many units it removed. Pinned by TestForgetRetiredDrives_*.
+func (h *SudoHostOps) ForgetRetiredDrives(ctx context.Context, intent IntentReader) int {
+	if intent == nil {
+		return 0 // no intent store → we cannot know a drive was retired; forget nothing
+	}
+	entries, err := os.ReadDir(h.unitDir)
+	if err != nil {
+		h.logger.Warn("storage: forget retired drives — cannot read unit dir", "dir", h.unitDir, "err", err)
+		return 0
+	}
+	mounted := map[string]bool{}
+	if ms, merr := h.host.Mounts(); merr == nil {
+		for _, m := range ms {
+			mounted[m.MountPoint] = true
+		}
+	} else {
+		h.logger.Warn("storage: forget retired drives — mount table unreadable; forgetting nothing", "err", merr)
+		return 0
+	}
+	removed := 0
+	for _, e := range entries {
+		name := e.Name()
+		if !strings.HasSuffix(name, ".mount") {
+			continue
+		}
+		data, rerr := os.ReadFile(filepath.Join(h.unitDir, name))
+		if rerr != nil {
+			continue
+		}
+		spec, ok := parseFelhomMountUnit(string(data))
+		if !ok {
+			continue // not one of our by-UUID drive units (network units are refused by the parser)
+		}
+		durableID := "uuid:" + spec.UUID
+		if intent.Get(durableID) != IntentDecommissioned {
+			continue
+		}
+		if mounted[spec.Where] {
+			h.logger.Debug("storage: decommissioned drive still mounted — keeping its unit", "where", spec.Where)
+			continue
+		}
+		if _, derr := ResolveStorageDevice(durableID); derr == nil {
+			h.logger.Debug("storage: decommissioned drive is present — keeping its unit (re-enroll stays one click)", "where", spec.Where)
+			continue
+		}
+		if h.unitEnabled(name) {
+			if err := h.run(ctx, h.bins.Systemctl, "disable", "--", name); err != nil {
+				h.logger.Warn("storage: forget retired drive — disable failed", "where", spec.Where, "unit", name, "err", err)
+				continue
+			}
+		}
+		if err := h.run(ctx, h.bins.PrivApply, "unit-forget", name); err != nil {
+			h.logger.Warn("storage: forget retired drive — unit removal refused or failed", "where", spec.Where, "unit", name, "err", err)
+			continue
+		}
+		removed++
+		h.logger.Info("storage: forgot a decommissioned drive that is gone — its mount unit is removed",
+			"name", spec.Name, "where", spec.Where, "uuid", spec.UUID, "unit", name)
+	}
+	if removed > 0 {
+		if err := h.run(ctx, h.bins.Systemctl, "daemon-reload"); err != nil {
+			h.logger.Warn("storage: daemon-reload after forgetting retired drives failed", "err", err)
+		}
+	}
+	return removed
+}

internal/storage/forget_retired_test.go

@@ -0,0 +1,155 @@
+package storage
+
+import (
+	"context"
+	"io"
+	"os"
+	"path/filepath"
+	"reflect"
+	"testing"
+)
+
+// forgetRunner records every privileged call and plays the root half of `felhom-priv-apply unit-forget`: the unit file
+// disappears from the unit dir, so the CONSEQUENCE (the drive is no longer listed) is observable, not just the calls.
+type forgetRunner struct {
+	unitDir string
+	calls   [][]string
+}
+
+func (r *forgetRunner) RunStdin(ctx context.Context, _ io.Reader, name string, args ...string) ([]byte, []byte, error) {
+	return r.Run(ctx, name, args...)
+}
+
+func (r *forgetRunner) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) {
+	r.calls = append(r.calls, append([]string{name}, args...))
+	if len(args) == 2 && args[0] == "unit-forget" {
+		_ = os.Remove(filepath.Join(r.unitDir, args[1]))
+	}
+	return nil, nil, nil
+}
+
+type forgetCase struct {
+	intent   DriveIntent
+	present  bool // the by-uuid link resolves
+	mounted  bool
+	enabled  bool // the wants-symlink exists
+	mountErr bool
+}
+
+const forgetUUID = "4ebfd36d-87d3-4b63-839e-a5a710fc799a"
+
+func runForget(t *testing.T, c forgetCase, intent IntentReader) (int, *forgetRunner, *RegistryKnownTargets, string) {
+	t.Helper()
+	unitDir := t.TempDir()
+	unit := "mnt-media_a.mount"
+	writeUnit(t, unitDir, unit, renderMountUnit(MountSpec{Name: forgetUUID, UUID: forgetUUID, Where: "/mnt/media_a", FSType: "ext4"}))
+	if c.enabled {
+		if err := os.MkdirAll(filepath.Join(unitDir, "multi-user.target.wants"), 0o755); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.Symlink(filepath.Join(unitDir, unit), filepath.Join(unitDir, "multi-user.target.wants", unit)); err != nil {
+			t.Fatal(err)
+		}
+	}
+	devRoot := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(devRoot, "by-uuid"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if c.present {
+		dev := filepath.Join(devRoot, "sdz1")
+		if err := os.WriteFile(dev, nil, 0o600); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.Symlink(dev, filepath.Join(devRoot, "by-uuid", forgetUUID)); err != nil {
+			t.Fatal(err)
+		}
+	}
+	withDevDiskRoot(t, devRoot)
+	hr := &fakeHostReader{}
+	if c.mounted {
+		hr.mounts = []Mount{{MountPoint: "/mnt/media_a", FSType: "ext4"}}
+	}
+	if c.mountErr {
+		hr.mountsErr = os.ErrPermission
+	}
+	rr := &forgetRunner{unitDir: unitDir}
+	ops := NewSudoHostOps(SudoHostOpsConfig{Runner: rr, Bins: Binaries{}.withDefaults(), UnitDir: unitDir,
+		StageDir: t.TempDir(), Host: hr, Logger: quietLogger()})
+	if intent == nil {
+		intent = &fakeIntent{m: map[string]DriveIntent{"uuid:" + forgetUUID: c.intent}}
+	}
+	n := ops.ForgetRetiredDrives(context.Background(), intent)
+	return n, rr, NewRegistryKnownTargets(unitDir, intent, quietLogger()), unitDir
+}
+
+// The R-1124 case: decommissioned, then unplugged. The unit is disabled and removed through the root checker, systemd
+// is reloaded, and the CONSEQUENCE — the agent no longer lists the drive — holds. RED-PROOF (P1/redproof-forget.txt):
+// with ForgetRetiredDrives' body replaced by `return 0` this test fails (the drive is still listed).
+func TestForgetRetiredDrives_DecommissionedAndGone_UnitRemovedAndNotListed(t *testing.T) {
+	n, rr, known, _ := runForget(t, forgetCase{intent: IntentDecommissioned, enabled: true}, nil)
+	if n != 1 {
+		t.Fatalf("removed = %d, want 1 (calls %v)", n, rr.calls)
+	}
+	pa := Binaries{}.withDefaults()
+	want := [][]string{
+		{pa.Systemctl, "disable", "--", "mnt-media_a.mount"},
+		{pa.PrivApply, "unit-forget", "mnt-media_a.mount"},
+		{pa.Systemctl, "daemon-reload"},
+	}
+	if !reflect.DeepEqual(rr.calls, want) {
+		t.Fatalf("calls = %v\nwant    %v", rr.calls, want)
+	}
+	got, err := known.Known(context.Background())
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(got) != 0 {
+		t.Fatalf("the forgotten drive is still listed: %+v", got)
+	}
+}
+
+// Already disabled (the Tester 1 shape after a manual disable): no disable call, the removal still runs.
+func TestForgetRetiredDrives_AlreadyDisabled_NoDisableCall(t *testing.T) {
+	n, rr, _, _ := runForget(t, forgetCase{intent: IntentDecommissioned}, nil)
+	pa := Binaries{}.withDefaults()
+	want := [][]string{{pa.PrivApply, "unit-forget", "mnt-media_a.mount"}, {pa.Systemctl, "daemon-reload"}}
+	if n != 1 || !reflect.DeepEqual(rr.calls, want) {
+		t.Fatalf("n=%d calls=%v want %v", n, rr.calls, want)
+	}
+}
+
+// Everything that must KEEP its unit: a decommissioned drive still plugged in (re-enroll is one click — H3), one still
+// mounted, an enrolled or ejected drive that is merely absent (it comes back), a drive with no recorded intent, and an
+// unreadable mount table. No privileged call at all, and the drive stays listed where it was listed.
+// RED-PROOF: with the present-drive check removed, the "present" case calls unit-forget and fails.
+func TestForgetRetiredDrives_KeepsEveryOtherUnit(t *testing.T) {
+	for name, c := range map[string]forgetCase{
+		"decommissioned but present": {intent: IntentDecommissioned, present: true, enabled: true},
+		"decommissioned but mounted": {intent: IntentDecommissioned, mounted: true},
+		"enrolled and absent":        {intent: IntentEnrolled},
+		"ejected and absent":         {intent: IntentEjected},
+		"no intent":                  {intent: IntentNew},
+		"mount table unreadable":     {intent: IntentDecommissioned, mountErr: true},
+	} {
+		t.Run(name, func(t *testing.T) {
+			n, rr, _, unitDir := runForget(t, c, nil)
+			if n != 0 || len(rr.calls) != 0 {
+				t.Fatalf("n=%d calls=%v — this unit must be kept", n, rr.calls)
+			}
+			if _, err := os.Stat(filepath.Join(unitDir, "mnt-media_a.mount")); err != nil {
+				t.Fatalf("unit file gone: %v", err)
+			}
+		})
+	}
+}
+
+// No intent store on the host (open failure): nothing is known to be retired, so nothing is forgotten.
+func TestForgetRetiredDrives_NilIntent_ForgetsNothing(t *testing.T) {
+	unitDir := t.TempDir()
+	writeUnit(t, unitDir, "mnt-media_a.mount", renderMountUnit(MountSpec{Name: "x", UUID: forgetUUID, Where: "/mnt/media_a"}))
+	rr := &forgetRunner{unitDir: unitDir}
+	ops := NewSudoHostOps(SudoHostOpsConfig{Runner: rr, UnitDir: unitDir, StageDir: t.TempDir(), Host: &fakeHostReader{}, Logger: quietLogger()})
+	if n := ops.ForgetRetiredDrives(context.Background(), nil); n != 0 || len(rr.calls) != 0 {
+		t.Fatalf("n=%d calls=%v", n, rr.calls)
+	}
+}

internal/thincheck/thincheck.go

@@ -0,0 +1,243 @@
+// Package thincheck finds an ORPHAN thin device: one the LVM thin pool holds that LVM itself no longer knows (R-1012).
+//
+// A full host root while vzdump takes its storage snapshot can leave such a device behind (demo-hp 2026-10-14: the
+// pool held devices 1, 2 and 4; LVM knew 1 and 2). From then on every snapshot backup of that guest fails at the
+// snapshot step („Creation of new snapshot 4 of device 2 failed") and nothing in the product said why. This package
+// NAMES the orphan, in plain words, in the backup's failure — it never removes one: the repair
+// (`dmsetup message <pool>-tpool 0 "delete <id>"`) stays a human step, decided by the operator (`03` whole-guest
+// backup; runbook `whole-guest-backup-orphan-thin.md`).
+//
+// The device list comes from the root helper `felhom-thin-check <vg> <pool>` (config bundle), which reads the pool
+// through a metadata snapshot and LVM's list before and after. An id is an orphan only when the pool holds it and
+// NEITHER LVM list names it, on two reads a few seconds apart — so a volume being created at that moment is never
+// reported.
+package thincheck
+
+import (
+	"context"
+	"fmt"
+	"log/slog"
+	"sort"
+	"strconv"
+	"strings"
+	"time"
+
+	"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
+)
+
+// HelperPath is where the config bundle installs the root helper.
+const HelperPath = "/usr/local/sbin/felhom-thin-check"
+
+// Pool names one LVM thin pool.
+type Pool struct{ VG, Pool string }
+
+func (p Pool) String() string { return p.VG + "/" + p.Pool }
+
+// Reading is one helper run, parsed.
+type Reading struct {
+	PoolIDs   []int
+	LVMBefore map[int]string
+	LVMAfter  map[int]string
+}
+
+// Parse reads the helper's output. ok=false unless the closing "ok" line is there (a cut-off run is no reading).
+func Parse(out string) (Reading, bool) {
+	r := Reading{LVMBefore: map[int]string{}, LVMAfter: map[int]string{}}
+	done := false
+	for _, line := range strings.Split(out, "\n") {
+		f := strings.Fields(line)
+		switch {
+		case len(f) == 1 && f[0] == "ok":
+			done = true
+		case len(f) == 2 && f[0] == "pool":
+			if id, err := strconv.Atoi(f[1]); err == nil {
+				r.PoolIDs = append(r.PoolIDs, id)
+			}
+		case len(f) == 3 && (f[0] == "lvm-before" || f[0] == "lvm-after"):
+			id, err := strconv.Atoi(f[1])
+			if err != nil {
+				continue
+			}
+			if f[0] == "lvm-before" {
+				r.LVMBefore[id] = f[2]
+			} else {
+				r.LVMAfter[id] = f[2]
+			}
+		}
+	}
+	return r, done
+}
+
+// Orphans returns the ids the pool holds that neither LVM list names, sorted.
+func (r Reading) Orphans() []int {
+	var out []int
+	for _, id := range r.PoolIDs {
+		_, b := r.LVMBefore[id]
+		_, a := r.LVMAfter[id]
+		if !b && !a {
+			out = append(out, id)
+		}
+	}
+	sort.Ints(out)
+	return out
+}
+
+// Runner is the privileged exec surface (proxmox.ExecRunner in sudo mode).
+type Runner interface {
+	Run(ctx context.Context, name string, args ...string) ([]byte, []byte, error)
+}
+
+// StorageLister is the read the checker needs to map a guest's volumes to their thin pools (*proxmox.Client).
+type StorageLister interface {
+	GuestConfig(ctx context.Context, vmid int) (proxmox.GuestConfig, error)
+	ListStorage(ctx context.Context) ([]proxmox.Storage, error)
+}
+
+// Checker runs the check for one guest's pools.
+type Checker struct {
+	API    StorageLister
+	Runner Runner
+	Helper string        // "" → HelperPath
+	Pause  time.Duration // between the two reads when the first finds an orphan; 0 → 3 s
+	Logger *slog.Logger
+}
+
+// Finding is one pool with orphans.
+type Finding struct {
+	Pool    Pool
+	Orphans []int
+}
+
+// PoolsOf maps the guest's volume-backed disks (rootfs, mpN) to the lvmthin pools they live on.
+func (c *Checker) PoolsOf(ctx context.Context, vmid int) ([]Pool, error) {
+	cfg, err := c.API.GuestConfig(ctx, vmid)
+	if err != nil {
+		return nil, err
+	}
+	stores, err := c.API.ListStorage(ctx)
+	if err != nil {
+		return nil, err
+	}
+	thin := map[string]Pool{}
+	for _, s := range stores {
+		if s.Type == "lvmthin" && s.VGName != "" && s.ThinPool != "" {
+			thin[s.Storage] = Pool{VG: s.VGName, Pool: s.ThinPool}
+		}
+	}
+	vols := []string{cfg.RootFS}
+	mps := cfg.MountPoints()
+	keys := make([]string, 0, len(mps))
+	for k := range mps {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	for _, k := range keys {
+		vols = append(vols, mps[k])
+	}
+	seen := map[Pool]bool{}
+	var out []Pool
+	for _, v := range vols {
+		store, _, ok := strings.Cut(strings.Split(v, ",")[0], ":")
+		if !ok || strings.HasPrefix(store, "/") {
+			continue // a bind mount (a path) is no volume
+		}
+		if p, isThin := thin[store]; isThin && !seen[p] {
+			seen[p] = true
+			out = append(out, p)
+		}
+	}
+	return out, nil
+}
+
+// read runs the helper once.
+func (c *Checker) read(ctx context.Context, p Pool) (Reading, error) {
+	helper := c.Helper
+	if helper == "" {
+		helper = HelperPath
+	}
+	out, stderr, err := c.Runner.Run(ctx, helper, p.VG, p.Pool)
+	if err != nil {
+		return Reading{}, fmt.Errorf("felhom-thin-check %s: %w: %s", p, err, strings.TrimSpace(string(stderr)))
+	}
+	r, ok := Parse(string(out))
+	if !ok {
+		return Reading{}, fmt.Errorf("felhom-thin-check %s: the output has no closing ok line", p)
+	}
+	return r, nil
+}
+
+// Check returns the pools of vmid that hold an orphan (two reads, intersected). A pool that cannot be read is logged
+// and left out: the check FAILS OPEN — it explains a failure, it never stops a backup.
+func (c *Checker) Check(ctx context.Context, vmid int) []Finding {
+	logger := c.Logger
+	if logger == nil {
+		logger = slog.Default()
+	}
+	pools, err := c.PoolsOf(ctx, vmid)
+	if err != nil {
+		logger.Warn("thincheck: cannot map the guest's disks to thin pools — not checked", "vmid", vmid, "err", err)
+		return nil
+	}
+	pause := c.Pause
+	if pause <= 0 {
+		pause = 3 * time.Second
+	}
+	var found []Finding
+	for _, p := range pools {
+		first, err := c.read(ctx, p)
+		if err != nil {
+			logger.Warn("thincheck: thin pool not read — not checked", "vmid", vmid, "pool", p.String(), "err", err)
+			continue
+		}
+		o1 := first.Orphans()
+		if len(o1) == 0 {
+			logger.Debug("thincheck: thin pool and LVM agree", "vmid", vmid, "pool", p.String(), "devices", len(first.PoolIDs))
+			continue
+		}
+		select {
+		case <-ctx.Done():
+			return found
+		case <-time.After(pause):
+		}
+		second, err := c.read(ctx, p)
+		if err != nil {
+			logger.Warn("thincheck: second read failed — not reported", "vmid", vmid, "pool", p.String(), "err", err)
+			continue
+		}
+		in2 := map[int]bool{}
+		for _, id := range second.Orphans() {
+			in2[id] = true
+		}
+		var both []int
+		for _, id := range o1 {
+			if in2[id] {
+				both = append(both, id)
+			}
+		}
+		if len(both) > 0 {
+			found = append(found, Finding{Pool: p, Orphans: both})
+			logger.Warn("thincheck: ORPHAN thin device(s) in the pool — LVM does not know them; NOT removed (a human step)",
+				"vmid", vmid, "pool", p.String(), "orphans", both)
+		}
+	}
+	return found
+}
+
+// Describe is the operator sentence for a backup failure. Plain words; names the pool and the ids; says nothing was
+// removed and where the repair is written down.
+func Describe(f []Finding) string {
+	if len(f) == 0 {
+		return ""
+	}
+	var parts []string
+	for _, x := range f {
+		ids := make([]string, len(x.Orphans))
+		for i, id := range x.Orphans {
+			ids[i] = strconv.Itoa(id)
+		}
+		parts = append(parts, fmt.Sprintf("thin pool %s holds device %s that LVM does not know", x.Pool, strings.Join(ids, ", ")))
+	}
+	return "probable cause: " + strings.Join(parts, "; ") +
+		" (an orphan snapshot, usually left when the host disk filled during a backup). Snapshot backups on this pool " +
+		"fail until an operator removes it; Felhom did not remove anything. Repair: runbook whole-guest-backup-orphan-thin.md"
+}

internal/thincheck/thincheck_test.go

@@ -0,0 +1,184 @@
+package thincheck
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"io"
+	"log/slog"
+	"os/exec"
+	"path/filepath"
+	"reflect"
+	"runtime"
+	"strings"
+	"testing"
+
+	"gitea.dooplex.hu/admin/felhom-agent/internal/proxmox"
+)
+
+// The helper's output on demo-hp 2026-10-14, before the repair: the pool held 1, 2 and 4; LVM knew 1 and 2
+// (audits/power-cut-fixes-2026-10-14/E/thin-ls.txt).
+const orphanOut = `lvm-before 1 vm-9201-disk-0
+lvm-before 2 vm-9201-disk-1
+pool 1
+pool 2
+pool 4
+lvm-after 1 vm-9201-disk-0
+lvm-after 2 vm-9201-disk-1
+ok
+`
+
+const healthyOut = `lvm-before 1 vm-9201-disk-0
+lvm-before 2 vm-9201-disk-1
+pool 1
+pool 2
+lvm-after 1 vm-9201-disk-0
+lvm-after 2 vm-9201-disk-1
+ok
+`
+
+func quiet() *slog.Logger { return slog.New(slog.NewTextHandler(io.Discard, nil)) }
+
+func TestParse_TheOrphanOf20261014(t *testing.T) {
+	r, ok := Parse(orphanOut)
+	if !ok {
+		t.Fatal("a complete reading was not accepted")
+	}
+	if got := r.Orphans(); !reflect.DeepEqual(got, []int{4}) {
+		t.Fatalf("orphans = %v, want [4]", got)
+	}
+	h, _ := Parse(healthyOut)
+	if got := h.Orphans(); len(got) != 0 {
+		t.Fatalf("a healthy pool reported orphans %v", got)
+	}
+}
+
+// A volume LVM created WHILE the pool was read appears only in the after-list: never an orphan.
+func TestOrphans_AVolumeCreatedDuringTheReadIsNotAnOrphan(t *testing.T) {
+	r, _ := Parse(strings.Replace(orphanOut, "lvm-after 2 vm-9201-disk-1\n", "lvm-after 2 vm-9201-disk-1\nlvm-after 4 snap_vm-9201-disk-1_vzdump\n", 1))
+	if got := r.Orphans(); len(got) != 0 {
+		t.Fatalf("orphans = %v, want none", got)
+	}
+}
+
+// A run cut off before its closing "ok" (the helper died, the output is partial) is no reading at all.
+func TestParse_NoClosingLineIsNoReading(t *testing.T) {
+	if _, ok := Parse(strings.TrimSuffix(orphanOut, "ok\n")); ok {
+		t.Fatal("a cut-off run was accepted as a reading")
+	}
+}
+
+type fakeAPI struct {
+	cfg    proxmox.GuestConfig
+	stores []proxmox.Storage
+}
+
+func (f *fakeAPI) GuestConfig(context.Context, int) (proxmox.GuestConfig, error) { return f.cfg, nil }
+func (f *fakeAPI) ListStorage(context.Context) ([]proxmox.Storage, error)        { return f.stores, nil }
+
+func guest9201() *fakeAPI {
+	extra := map[string]json.RawMessage{
+		"mp0": json.RawMessage(`"local-lvm:vm-9201-disk-1,mp=/var/lib/felhom,backup=1,size=20G"`),
+		"mp8": json.RawMessage(`"/mnt/felhom-drives,mp=/mnt/felhom-drives"`),
+		"mp9": json.RawMessage(`"local:9201/vm-9201-disk-2.raw,mp=/x"`),
+	}
+	return &fakeAPI{
+		cfg: proxmox.GuestConfig{RootFS: "local-lvm:vm-9201-disk-0,size=8G", Extra: extra},
+		stores: []proxmox.Storage{
+			{Storage: "local", Type: "dir", Path: "/var/lib/vz"},
+			{Storage: "local-lvm", Type: "lvmthin", VGName: "pve", ThinPool: "data"},
+			{Storage: "felhom-pbs", Type: "pbs"},
+		},
+	}
+}
+
+type seqRunner struct {
+	outs  []string
+	errs  []error
+	calls [][]string
+}
+
+func (r *seqRunner) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) {
+	r.calls = append(r.calls, append([]string{name}, args...))
+	i := len(r.calls) - 1
+	var err error
+	if i < len(r.errs) {
+		err = r.errs[i]
+	}
+	if i < len(r.outs) {
+		return []byte(r.outs[i]), nil, err
+	}
+	return nil, nil, err
+}
+
+func TestPoolsOf_RootfsAndVolumeMountpointsOnly(t *testing.T) {
+	c := &Checker{API: guest9201(), Runner: &seqRunner{}, Logger: quiet()}
+	pools, err := c.PoolsOf(context.Background(), 9201)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !reflect.DeepEqual(pools, []Pool{{VG: "pve", Pool: "data"}}) {
+		t.Fatalf("pools = %v, want [pve/data] once (bind mounts and dir volumes are not thin)", pools)
+	}
+}
+
+// The consequence: an orphan seen on BOTH reads becomes a finding whose sentence names the pool and the id, and says
+// nothing was removed. The only command ever run is the read-only helper with <vg> <pool>.
+// RED-PROOF (P1/redproof-thincheck.txt): with Orphans() returning nil this test fails.
+func TestCheck_OrphanOnBothReads_IsNamed(t *testing.T) {
+	rr := &seqRunner{outs: []string{orphanOut, orphanOut}}
+	c := &Checker{API: guest9201(), Runner: rr, Pause: 1, Logger: quiet()}
+	f := c.Check(context.Background(), 9201)
+	if len(f) != 1 || f[0].Pool.String() != "pve/data" || !reflect.DeepEqual(f[0].Orphans, []int{4}) {
+		t.Fatalf("findings = %+v", f)
+	}
+	want := [][]string{{HelperPath, "pve", "data"}, {HelperPath, "pve", "data"}}
+	if !reflect.DeepEqual(rr.calls, want) {
+		t.Fatalf("calls = %v, want only the read-only helper twice", rr.calls)
+	}
+	d := Describe(f)
+	for _, s := range []string{"thin pool pve/data holds device 4 that LVM does not know", "Felhom did not remove anything",
+		"whole-guest-backup-orphan-thin.md"} {
+		if !strings.Contains(d, s) {
+			t.Fatalf("Describe = %q, missing %q", d, s)
+		}
+	}
+}
+
+// An id that is gone on the second read (a volume mid-creation) is not reported; a healthy pool is read ONCE.
+func TestCheck_TransientOrHealthy_NothingReported(t *testing.T) {
+	rr := &seqRunner{outs: []string{orphanOut, healthyOut}}
+	if f := (&Checker{API: guest9201(), Runner: rr, Pause: 1, Logger: quiet()}).Check(context.Background(), 9201); len(f) != 0 {
+		t.Fatalf("a transient id was reported: %+v", f)
+	}
+	rr = &seqRunner{outs: []string{healthyOut}}
+	if f := (&Checker{API: guest9201(), Runner: rr, Logger: quiet()}).Check(context.Background(), 9201); len(f) != 0 || len(rr.calls) != 1 {
+		t.Fatalf("healthy: findings %+v, calls %v", f, rr.calls)
+	}
+	if Describe(nil) != "" {
+		t.Fatal("no finding must describe as empty")
+	}
+}
+
+// The helper refusing (a metadata snapshot held by someone else, exit 3) or failing: no finding, no panic — the check
+// fails open; it explains a failure, it never stops a backup.
+func TestCheck_HelperRefuses_FailsOpen(t *testing.T) {
+	rr := &seqRunner{errs: []error{errors.New("exit status 3")}}
+	if f := (&Checker{API: guest9201(), Runner: rr, Logger: quiet()}).Check(context.Background(), 9201); len(f) != 0 {
+		t.Fatalf("findings = %+v", f)
+	}
+}
+
+// The root helper's own tests (configs/test_felhom_thin_check.py) run with the Go suite.
+func TestRootHelperPythonSuite(t *testing.T) {
+	py, err := exec.LookPath("python3")
+	if err != nil {
+		t.Skip("python3 not found")
+	}
+	_, file, _, _ := runtime.Caller(0)
+	script := filepath.Join(filepath.Dir(file), "..", "..", "configs", "test_felhom_thin_check.py")
+	out, err := exec.Command(py, script).CombinedOutput()
+	if err != nil {
+		t.Fatalf("helper suite failed: %v\n%s", err, out)
+	}
+}