diff --git a/CHANGELOG.md b/CHANGELOG.md index 637354db..e5bf5c92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,11 +22,13 @@ a unix socket `/ctl/hold.sock`, 0600 in a 0700 dir, peer uid checked (SO_PEERCRED), nothing on TCP — not reachable through traefik, the LAN or an app container. Output: „Cleared by the RUNNING controller … no restart is needed". Only when no controller answers (no socket, or a stale one) does it edit `settings.json`, and it says so, - naming the restart INSIDE the guest (`pct exec -- systemctl restart felhom-controller-bootstrap.service`). The + naming the restart INSIDE the guest (`pct exec -- systemctl restart felhom-controller-bootstrap.service`); that + edit goes through the same door, so a waiting put-back is dropped from the marker there too. The flag's help names the exact guest command. `--restore-holds` (the list) is unchanged. Tests: `TestR1343_TheBoxCommandClearsThroughTheRunningController` (red with the socket skipped: „the hold came back after a later save of the running controller" — R-1343 reproduced), `TestR1343_TheBoxCommandFallsBackWhenNoControllerRuns`, - `TestHoldCtl_NotReachableFromOutside`, `TestHoldCtl_UnknownRequestIsRefused`, `TestHoldCtl_ANonSocketFileIsNotReplaced`. + `TestR1343_TheFallbackDropsAWaitingPutBackToo`, `TestHoldCtl_NotReachableFromOutside`, + `TestHoldCtl_UnknownRequestIsRefused`, `TestHoldCtl_ANonSocketFileIsNotReplaced`. - **R-1340 — the operator's „HELD STOPPED" mail names the real cause** (`cmd/controller/holdmail.go` `heldStoppedOperatorText`, built from the hold record). Hold writers now record `RestoreKind` (off-site / local), `MixedCause` (`definition_write` / `put_back` / `interrupted_put_back`), `FailedStep` (the put-back step that failed, diff --git a/controller/cmd/controller/holdclear.go b/controller/cmd/controller/holdclear.go index 01e15db0..57fc6a9a 100644 --- a/controller/cmd/controller/holdclear.go +++ b/controller/cmd/controller/holdclear.go @@ -238,18 +238,25 @@ func clearRestoreHoldCLI(dataDir, stack string, stdout, stderr io.Writer) int { fmt.Fprintf(stderr, "restore-hold: loading settings: %v\n", lerr) return 1 } - cleared, cerr := sett.ClearRestoreHold(stack) - if cerr != nil { - fmt.Fprintf(stderr, "restore-hold: clearing %s: %v\n", stack, cerr) - return 1 - } - if !cleared { + // The same door as the running controller's (a waiting put-back is dropped from the app-stop marker too), over + // this process's copy — the only copy while no controller runs. + g := backup.NewAppStopGuard(filepath.Join(dataDir, "appstop-state.json"), lg) + g.SetHolds(sett) + h, cerr := g.ClearHoldByOperator(stack) + switch { + case errors.Is(cerr, backup.ErrNotHeld): // "there was nothing to clear" is not success — a silent 0 here would let an operator believe they had // unblocked an app whose name they mistyped. fmt.Fprintf(stderr, "no restore hold in force for %q — nothing was changed\n", stack) return 2 + case cerr != nil: + fmt.Fprintf(stderr, "restore-hold: clearing %s: %v\n", stack, cerr) + return 1 + } + fmt.Fprintf(stdout, "restore hold cleared for %s in settings.json (no controller was running to ask; it was: %s).\n", stack, h.Why()) + if h.Reason == settings.HoldReasonPutBackPending { + fmt.Fprintf(stdout, " Its waiting put-back is dropped; the pre-restore copy stays on disk at %s.\n", orNone(h.SafetyDump)) } - fmt.Fprintf(stdout, "restore hold cleared for %s in settings.json (no controller was running to ask).\n", stack) fmt.Fprintf(stdout, " The controller reads it when it starts. If a controller IS running in this guest after all, restart it NOW,\n") fmt.Fprintf(stdout, " INSIDE THE GUEST — from the Proxmox host: pct exec -- systemctl restart felhom-controller-bootstrap.service\n") fmt.Fprintf(stdout, " — or its next settings save writes the hold back (R-1343).\n") diff --git a/controller/cmd/controller/holdclear_test.go b/controller/cmd/controller/holdclear_test.go index 0bde0630..e0a75d53 100644 --- a/controller/cmd/controller/holdclear_test.go +++ b/controller/cmd/controller/holdclear_test.go @@ -514,3 +514,46 @@ func TestD64_MainWiresTheNewPieces(t *testing.T) { } func timeAt(sec int) time.Time { return time.Date(2026, 10, 29, 12, 0, sec, 0, time.UTC) } + +// No controller runs: the box command's own clear drops a waiting put-back from the app-stop marker too (the same +// door), so the next start with the drive bound pours nothing back. +// RED-PROOF (2026-10-29): with the fallback clearing through settings.ClearRestoreHold alone (its first shape in +// 5b3b541), this fails on „a later controller start poured the old copy back". +func TestR1343_TheFallbackDropsAWaitingPutBackToo(t *testing.T) { + r := newD64Rig(t) + undo := filepath.Join(r.dir, "drive", "felhom-data", "backups", "restore-undo", "immich", "20261029T080000Z") + if err := os.MkdirAll(undo, 0o700); err != nil { + t.Fatal(err) + } + if err := r.guard.Begin("unit-restore:immich", backup.ReasonUnitRestore, []string{"immich"}); err != nil { + t.Fatal(err) + } + if err := r.guard.SetUndo("immich", undo); err != nil { + t.Fatal(err) + } + r.hold(t, settings.RestoreHold{Stack: "immich", Reason: settings.HoldReasonPutBackPending, SafetyDump: undo}) + var out, errb bytes.Buffer + if code := clearRestoreHoldCLI(r.dir, "immich", &out, &errb); code != 0 { + t.Fatalf("exit %d, stderr %q", code, errb.String()) + } + if !strings.Contains(out.String(), "waiting put-back is dropped") { + t.Errorf("output must say the put-back is dropped:\n%s", out.String()) + } + sett2, err := settings.Load(filepath.Join(r.dir, "settings.json"), log.New(io.Discard, "", 0)) + if err != nil { + t.Fatal(err) + } + poured := &recordingPutBacker{} + g2 := backup.NewAppStopGuard(filepath.Join(r.dir, "appstop-state.json"), r.logger) + g2.SetHolds(sett2) + g2.SetPutBacker(poured) + g2.SetStarter(nopStarter{}) + g2.SetUndoReachable(func(string, string) bool { return true }) + g2.Recover() + if len(poured.stacks) != 0 { + t.Fatalf("a later controller start poured the old copy back over the app: %v", poured.stacks) + } + if _, err := os.Stat(undo); err != nil { + t.Fatal("the copy must stay on disk") + } +}