From 37ffa3beb58f56295ecc8d12ac23d488a9add645 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Thu, 15 Oct 2026 10:25:08 +0200 Subject: [PATCH] Safe removal (R-1044), found on Tester 1 G4: the drive gate leaves a safely removed drive alone (no false RETURNED/ABSENT, no mail); presence = backing device, not the agent's state Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS --- CHANGELOG.md | 5 ++ controller/internal/web/intermediary.go | 4 ++ controller/internal/web/intermediary_test.go | 17 +++++++ controller/internal/web/media_events.go | 50 ++++++++++++++------ controller/internal/web/media_events_test.go | 25 ++++++++++ 5 files changed, 87 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b11d593..c91a784 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,11 @@ Not released yet (bake → vouch → floor, `09` §3 decision 247). Design: felh `TestRun_SecondRunWritesNothing` (red-proved: the measured repeated qBittorrent writes). - **Fixed on Tester 1 (Part G1):** qBittorrent 5 answers a good login with HTTP 204, not 200 — the wiring refused it (`login: HTTP 204`). Both logins accept 204; the test fake now answers 204 as the real app does (red-proved). +- **Fixed on Tester 1 (Part G4), two safe-removal faults:** (1) the periodic drive gate saw the drive still bound during + the 32-second removal and called it RETURNED, then ABSENT with a „disconnected" mail — a safely removed drive is now the + removal pass's alone (`TestPlanDriveGates_SkipsSafelyRemovedDrive`, red before the fix); (2) the removal pass read the + agent's `state`, which stays „attached" for a registered disk that is gone — presence is now a backing device + (`TestEjectStep_G4` from the two measured records, red-proved). - **Old Sonarr/Radarr installs repaired by themselves (R-1047)** — only when /media is no longer bound: same-path root folder, `moveFiles:false` editor, rescan, old root removed, Radarr's collections too. `TestRepairOldRoot_K7` (red-proved: moveFiles=true is caught). diff --git a/controller/internal/web/intermediary.go b/controller/internal/web/intermediary.go index adde44a..b76e5a2 100644 --- a/controller/internal/web/intermediary.go +++ b/controller/internal/web/intermediary.go @@ -266,6 +266,10 @@ func planDriveGates(paths []settings.StoragePath, disks []agentapi.DiskInfo) []g if sp.Decommissioned { continue } + // R-1044: a SAFELY REMOVED drive is reconcileEjectedDrives' alone (TestPlanDriveGates_SkipsSafelyRemovedDrive). + if sp.EjectedAt != "" { + continue + } // NAS network storage is NOT a drive — the agent never lists it in /disks, so the drive-absent gate // would falsely see it "absent" (present[sp.Path]==false) and STOP its apps. A NAS blip is // recoverable (the agent's per-share liveness → a warning badge), never the drive stop-cascade. diff --git a/controller/internal/web/intermediary_test.go b/controller/internal/web/intermediary_test.go index 87e95f8..de31fef 100644 --- a/controller/internal/web/intermediary_test.go +++ b/controller/internal/web/intermediary_test.go @@ -291,3 +291,20 @@ func TestPlanDriveGates(t *testing.T) { t.Errorf("internal SSD/system path must NEVER be gated (only /mnt/felhom-drives/ externals)") } } + +// R-1044 (found on Tester 1, Part G4, 2026-10-15): a SAFELY REMOVED drive belongs to reconcileEjectedDrives only. The +// periodic gate saw it still bound during the 32-second removal and called it RETURNED (clearing the removal's +// disconnected mark), then ABSENT 30 s later — with a „disconnected" mail for a removal the household asked for. +// COMPANION RED-PROOF: drop the EjectedAt skip in planDriveGates → both cases below produce an action. Restored. +func TestPlanDriveGates_SkipsSafelyRemovedDrive(t *testing.T) { + p := "/mnt/felhom-drives/media_d" + removed := settings.StoragePath{Path: p, Disconnected: true, EjectedAt: "2026-10-15T08:10:20Z"} + still := []agentapi.DiskInfo{{GuestPath: p, MountPath: "/mnt/media_d", State: "attached", BoundUnderParent: true}} + if a := planDriveGates([]settings.StoragePath{removed}, still); len(a) != 0 { + t.Fatalf("a safely removed drive still bound during the removal must not be RETURNED by the gate: %+v", a) + } + removed.Disconnected = false + if a := planDriveGates([]settings.StoragePath{removed}, nil); len(a) != 0 { + t.Fatalf("a safely removed drive that is gone must not be gated ABSENT (no mail for a safe removal): %+v", a) + } +} diff --git a/controller/internal/web/media_events.go b/controller/internal/web/media_events.go index 80770a2..e5f86f9 100644 --- a/controller/internal/web/media_events.go +++ b/controller/internal/web/media_events.go @@ -325,21 +325,13 @@ func (s *Server) reconcileEjectedDrives(ctx context.Context, agent *agentapi.Cli if sp.EjectedAt == "" { continue } - var disk *agentapi.DiskInfo - for i := range disks { - if (sp.EjectDurableID != "" && disks[i].DurableID == sp.EjectDurableID) || (sp.EjectDurableID == "" && disks[i].GuestPath == sp.Path) { - disk = &disks[i] - } - } - attached := disk != nil && disk.State == "attached" - if !sp.EjectSeenGone { - if !attached { - _ = s.settings.SetEjectSeenGone(sp.Path) - s.logger.Printf("[INFO] [media] safely removed drive %s: its disk has left the box", sp.Path) - } + step, disk := ejectStep(sp, disks) + switch step { + case ejectWaitGone, ejectWaitBack: continue - } - if !attached { + case ejectGone: + _ = s.settings.SetEjectSeenGone(sp.Path) + s.logger.Printf("[INFO] [media] safely removed drive %s: its disk has left the box", sp.Path) continue } if !disk.BoundUnderParent && disk.MountPath != "" { @@ -420,3 +412,33 @@ var plexClaim = func(code string) (int, error) { resp.Body.Close() return resp.StatusCode, nil } + +// The safe-removal states (R-1044). +const ( + ejectWaitGone = iota // removed, the disk is still in the box + ejectGone // the disk has just been seen gone + ejectWaitBack // gone, not back yet + ejectBack // back: re-attach and clear +) + +// ejectStep is the PURE decision of reconcileEjectedDrives. A disk is PHYSICALLY present when the agent reports a backing +// device: its `state` stays „attached" for a registered disk that is gone (measured on Tester 1, Part G4: Media D pulled, +// state attached, backing_device "", total_bytes 0). Pinned by TestEjectStep_G4. +func ejectStep(sp settings.StoragePath, disks []agentapi.DiskInfo) (int, *agentapi.DiskInfo) { + var disk *agentapi.DiskInfo + for i := range disks { + if (sp.EjectDurableID != "" && disks[i].DurableID == sp.EjectDurableID) || (sp.EjectDurableID == "" && disks[i].GuestPath == sp.Path) { + disk = &disks[i] + } + } + present := disk != nil && disk.BackingDevice != "" + switch { + case !sp.EjectSeenGone && present: + return ejectWaitGone, disk + case !sp.EjectSeenGone: + return ejectGone, disk + case !present: + return ejectWaitBack, disk + } + return ejectBack, disk +} diff --git a/controller/internal/web/media_events_test.go b/controller/internal/web/media_events_test.go index 36a6532..722021d 100644 --- a/controller/internal/web/media_events_test.go +++ b/controller/internal/web/media_events_test.go @@ -3,6 +3,9 @@ package web import ( "testing" "time" + + "gitea.dooplex.hu/admin/felhom-controller/internal/agentapi" + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" ) // „Later" is tonight 03:30 Budapest time — or tomorrow's when it is already past 03:30. @@ -18,3 +21,25 @@ func TestNextMediaLater(t *testing.T) { } } } + +// R-1044, the records measured on Tester 1 (Part G4): a pulled, registered disk keeps state „attached" with an EMPTY +// backing device; the decision must read the backing device. COMPANION RED-PROOF: use `disk.State == "attached"` as +// presence → the „gone" case fails (the box never notices the disk left — what rc.2 did on Tester 1). Restored. +func TestEjectStep_G4(t *testing.T) { + sp := settings.StoragePath{Path: "/mnt/felhom-drives/media_d", EjectedAt: "2026-10-15T08:10:20Z", EjectDurableID: "uuid:8a31"} + in := agentapi.DiskInfo{DurableID: "uuid:8a31", State: "attached", BackingDevice: "/dev/sdd", MountPath: "/mnt/media_d", TotalBytes: 21474836480} + gone := agentapi.DiskInfo{DurableID: "uuid:8a31", State: "attached", BackingDevice: "", MountPath: "/mnt/media_d"} + if st, _ := ejectStep(sp, []agentapi.DiskInfo{in}); st != ejectWaitGone { + t.Fatalf("still in the box right after the removal: want wait-gone, got %d", st) + } + if st, _ := ejectStep(sp, []agentapi.DiskInfo{gone}); st != ejectGone { + t.Fatalf("pulled (state attached, no backing device): want gone, got %d", st) + } + sp.EjectSeenGone = true + if st, _ := ejectStep(sp, []agentapi.DiskInfo{gone}); st != ejectWaitBack { + t.Fatalf("still gone: want wait-back, got %d", st) + } + if st, d := ejectStep(sp, []agentapi.DiskInfo{in}); st != ejectBack || d == nil { + t.Fatalf("plugged back: want back, got %d", st) + } +}