R-1256: four restore edges (review round 2 HUNT-1 H4/H5/H6/H8) — each test seen failing first

H4: a failed definition write in the off-site restore writes the live definition back before the start;
    a failed write-back holds the app (restore_mixed).
H5: a put-back whose stop fails pours nothing, keeps the copy and holds the app (off-site, local, power-cut);
    a failed definition write-back keeps the copy (D20); a failed 'volume rm -f' fails that volume (no merge).
H6: TryBeginRestoreOp/RestoreBusy: check every operation and record the restore in one step under m.mu;
    every web handler and the kept loads begin through it; install-time kept load asks about the mirror too.
H8: SweepUnusedRestoreCopies every 6 h: removes a copy only with no hold, no marker naming it, no restore
    running, and older than 14 days.

Commit only; rides the next controller release.

controller/internal/web/handlers.go

@@ -1894,7 +1894,10 @@ func (s *Server) backupRestoreHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	s.logger.Printf("[WARN] [web] Restore requested (async): stack=%s, snapshot=%s from %s", stackName, snapshotID, r.RemoteAddr)
-	s.backupMgr.BeginRestoreOp("restore", stackName)
+	if msg, ok := s.beginRestoreOp("restore", stackName); !ok {
+		http.Redirect(w, r, "/backups/restore?flash_error="+url.QueryEscape(msg), http.StatusFound)
+		return
+	}
 	go func() {
 		start := time.Now()
 		// Phase 2b: restore from the app's recovery unit (recovers secrets from the guest, fail-closed
@@ -2236,7 +2239,10 @@ func (s *Server) backupTier2RestoreHandler(w http.ResponseWriter, r *http.Reques
 	}
 
 	s.logger.Printf("[WARN] [web] Tier-2 file restore requested (async): stack=%s from %s", stackName, r.RemoteAddr)
-	s.backupMgr.BeginRestoreOp("tier2-restore", stackName)
+	if msg, ok := s.beginRestoreOp("tier2-restore", stackName); !ok {
+		http.Redirect(w, r, "/backups/apps?flash_error="+url.QueryEscape(msg), http.StatusFound)
+		return
+	}
 	go func() {
 		n, err := s.backupMgr.RestoreTier2Files(stackName)
 		if err != nil {
@@ -2311,7 +2317,10 @@ func (s *Server) backupTier2UnitRestoreHandler(w http.ResponseWriter, r *http.Re
 	// (R-538), which is what left such an app with no way back from the second drive.
 	if s.backupMgr.HasDriveFileLegs(stackName) {
 		s.logger.Printf("[WARN] [web] Tier-2 WHOLE restore requested (async, files + database): stack=%s from %s", stackName, r.RemoteAddr)
-		s.backupMgr.BeginRestoreOp("tier2-whole-restore", stackName)
+		if msg, ok := s.beginRestoreOp("tier2-whole-restore", stackName); !ok {
+			http.Redirect(w, r, "/backups/apps?flash_error="+url.QueryEscape(msg), http.StatusFound)
+			return
+		}
 		go func() {
 			start := time.Now()
 			res, err := s.backupMgr.RestoreTier2Whole(stackName)
@@ -2346,7 +2355,10 @@ func (s *Server) backupTier2UnitRestoreHandler(w http.ResponseWriter, r *http.Re
 	}
 
 	s.logger.Printf("[WARN] [web] Tier-2 UNIT restore requested (async, OVERWRITES live data): stack=%s from %s", stackName, r.RemoteAddr)
-	s.backupMgr.BeginRestoreOp("tier2-unit-restore", stackName)
+	if msg, ok := s.beginRestoreOp("tier2-unit-restore", stackName); !ok {
+		http.Redirect(w, r, "/backups/apps?flash_error="+url.QueryEscape(msg), http.StatusFound)
+		return
+	}
 	go func() {
 		start := time.Now()
 		res, err := s.backupMgr.RestoreTier2Unit(stackName)

controller/internal/web/kept_copies.go

@@ -106,7 +106,10 @@ func (s *Server) keptUnitRestoreHandler(w http.ResponseWriter, r *http.Request)
 		return
 	}
 	s.logger.Printf("[WARN] [web] Restore of %s from a set-aside unit requested (async): %s from %s", k.Source, k.Path, r.RemoteAddr)
-	s.backupMgr.BeginRestoreOp("restore", k.Source)
+	if msg, ok := s.beginRestoreOp("restore", k.Source); !ok {
+		http.Redirect(w, r, "/backups/restore?flash_error="+url.QueryEscape(msg), http.StatusFound)
+		return
+	}
 	go func() {
 		app, res, err := s.backupMgr.RestoreSetAsideUnit(id)
 		if err != nil {

controller/internal/web/kept_handlers.go

@@ -217,16 +217,23 @@ func (s *Server) keptLoadHandler(w http.ResponseWriter, r *http.Request) {
 		_, err := s.stackMgr.RestoreKeptFiles(it)
 		return err
 	}
+	// R-1256 H6: the load begins in the same step as the check for every other operation; a refusal moves nothing
+	// (the dated folder's files move back inside the operation, as prepare).
+	var lErr error
 	if c.Tier == backup.KeptTierOffsite {
 		s.logger.Printf("[INFO] [web] kept LOAD %s: %s from the off-site snapshot %s (from %s)", it.App, it.Path, c.SnapshotID, r.RemoteAddr)
-		s.backupMgr.LoadKeptOffsite(app, it.Drive, c, moveBack, okMsg, failMsg, after)
+		lErr = s.backupMgr.LoadKeptOffsite(app, it.Drive, c, moveBack, okMsg, failMsg, after)
 	} else {
-		if err := moveBack(); err != nil {
-			s.keptRedirect(w, r, "flash_error", s.errText(r, err))
-			return
-		}
 		s.logger.Printf("[INFO] [web] kept LOAD %s: %s from %s (from %s)", it.App, it.Path, c.UnitDir, r.RemoteAddr)
-		s.backupMgr.LoadKeptApp(app, c.UnitDir, okMsg, failMsg, after)
+		lErr = s.backupMgr.LoadKeptApp(app, c.UnitDir, moveBack, okMsg, failMsg, after)
+	}
+	if lErr != nil {
+		msg, _ := s.restoreOpBlocked()
+		if msg == "" { // the other operation ended in the meantime
+			msg, _ = s.restoreBusyMessage(backup.RestoreBusyRestore, "")
+		}
+		s.keptRedirect(w, r, "flash_error", msg)
+		return
 	}
 	http.Redirect(w, r, "/backups/restore?"+flashQuery("flash", "flash.restore.started"), http.StatusFound)
 }

controller/internal/web/offbox_handlers.go

@@ -450,7 +450,10 @@ func (s *Server) offboxRestoreHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	full := mode == "full"
-	s.backupMgr.BeginRestoreOp("offbox-restore", app)
+	if msg, ok := s.beginRestoreOp("offbox-restore", app); !ok {
+		offboxRedirectTo(w, r, restoreWizardPath(app), msg, true)
+		return
+	}
 	go func() {
 		ctx, cancel := context.WithTimeout(context.Background(), 30*time.Minute)
 		defer cancel()
@@ -538,7 +541,10 @@ func (s *Server) offboxReconstituteHandler(w http.ResponseWriter, r *http.Reques
 	// data"; this one answers "yes, into a different place than the backup recorded". One checkbox
 	// carrying both would be the two-decisions-one-button shape R-48 removed from this surface.
 	ackPlacement := r.FormValue("ack_placement") == "1"
-	s.backupMgr.BeginRestoreOp("offbox-reconstitute", app)
+	if msg, ok := s.beginRestoreOp("offbox-reconstitute", app); !ok {
+		offboxRedirectTo(w, r, restoreWizardPath(app), msg, true)
+		return
+	}
 	go func() {
 		ctx, cancel := context.WithTimeout(context.Background(), 60*time.Minute)
 		defer cancel()
@@ -678,7 +684,10 @@ func (s *Server) offboxPlaceHandler(w http.ResponseWriter, r *http.Request) {
 		offboxRedirectTo(w, r, restoreWizardPath(app), offsiteScratchIncompleteKey, true)
 		return
 	}
-	s.backupMgr.BeginRestoreOp("offbox-place", app)
+	if msg, ok := s.beginRestoreOp("offbox-place", app); !ok {
+		offboxRedirectTo(w, r, restoreWizardPath(app), msg, true)
+		return
+	}
 	go func() {
 		ctx, cancel := context.WithTimeout(context.Background(), 30*time.Minute)
 		defer cancel()
@@ -711,7 +720,10 @@ func (s *Server) sharesRestoreHandler(w http.ResponseWriter, r *http.Request) {
 		offboxRedirectTo(w, r, "/backups/restore", msg, true)
 		return
 	}
-	s.backupMgr.BeginRestoreOp("shares-restore", backup.SharesDisplayName)
+	if msg, ok := s.beginRestoreOp("shares-restore", backup.SharesDisplayName); !ok {
+		offboxRedirectTo(w, r, "/backups/restore", msg, true)
+		return
+	}
 	go func() {
 		ctx, cancel := context.WithTimeout(context.Background(), 30*time.Minute)
 		defer cancel()
@@ -737,7 +749,10 @@ func (s *Server) sharesPlaceHandler(w http.ResponseWriter, r *http.Request) {
 		offboxRedirectTo(w, r, "/backups/restore", msg, true)
 		return
 	}
-	s.backupMgr.BeginRestoreOp("shares-place", backup.SharesDisplayName)
+	if msg, ok := s.beginRestoreOp("shares-place", backup.SharesDisplayName); !ok {
+		offboxRedirectTo(w, r, "/backups/restore", msg, true)
+		return
+	}
 	go func() {
 		ctx, cancel := context.WithTimeout(context.Background(), 30*time.Minute)
 		defer cancel()

controller/internal/web/r1256_begin_restore_test.go

@@ -0,0 +1,60 @@
+package web
+
+import (
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"gitea.dooplex.hu/admin/felhom-controller/internal/backup"
+)
+
+// R-1256 H6: every restore handler records its op through beginRestoreOp, which asks about every other operation in
+// the same step (backup.TryBeginRestoreOp). A handler that calls BeginRestoreOp directly re-opens the window between
+// the early refusal and the record. RED-PROOF: put one `s.backupMgr.BeginRestoreOp(...)` back → FAILS, naming the file.
+func TestR1256_H6_NoHandlerBeginsARestoreUnchecked(t *testing.T) {
+	files, _ := filepath.Glob("*.go")
+	for _, f := range files {
+		if strings.HasSuffix(f, "_test.go") {
+			continue
+		}
+		src, err := os.ReadFile(f)
+		if err != nil {
+			t.Fatal(err)
+		}
+		fs := token.NewFileSet()
+		af, err := parser.ParseFile(fs, f, src, 0)
+		if err != nil {
+			t.Fatal(err)
+		}
+		ast.Inspect(af, func(n ast.Node) bool {
+			if sel, ok := n.(*ast.SelectorExpr); ok && sel.Sel.Name == "BeginRestoreOp" {
+				t.Errorf("%s: a direct BeginRestoreOp — begin through s.beginRestoreOp (R-1256 H6)", fs.Position(sel.Pos()))
+			}
+			return true
+		})
+	}
+}
+
+// The handler's begin refuses with the household's sentence when a second-drive copy began after the early check.
+func TestR1256_H6_TheBeginRefusesWithTheCopySentence(t *testing.T) {
+	s, _, _ := newOffboxWebServer(t)
+	if _, blocked := s.restoreOpBlocked(); blocked {
+		t.Fatal("fixture: nothing runs")
+	}
+	done := backup.MarkTier2CopyForTest("immich")
+	msg, ok := s.beginRestoreOp("offbox-reconstitute", "immich")
+	done()
+	if ok || !strings.Contains(msg, "második meghajtóra") {
+		t.Fatalf("the begin must refuse with the copy named; ok=%v %q", ok, msg)
+	}
+	if s.backupMgr.RestoreStatus().Running {
+		t.Fatal("a refused begin recorded a restore")
+	}
+	if _, ok := s.beginRestoreOp("offbox-reconstitute", "immich"); !ok {
+		t.Fatal("after the copy the begin must work")
+	}
+}

controller/internal/web/restore_wizard.go

@@ -209,28 +209,50 @@ func (s *Server) restoreOpBlocked() (string, bool) {
 	if s.backupMgr == nil {
 		return "", false
 	}
-	if st := s.backupMgr.RestoreStatus(); restoreOpInFlight(st) {
+	busy, app := s.backupMgr.RestoreBusy()
+	return s.restoreBusyMessage(busy, app)
+}
+
+// beginRestoreOp records a restore op in the SAME step as the check for every other operation (R-1256 H6): the early
+// restoreOpBlocked answer can be stale by the time a handler has read its form, and a second-drive copy that began in
+// between ran beside the restore. A refusal returns the same sentence as restoreOpBlocked; the caller redirects with it
+// and starts nothing. Every restore handler begins through here — TestR1256_H6_NoHandlerBeginsARestoreUnchecked.
+func (s *Server) beginRestoreOp(op, stack string) (string, bool) {
+	busy, app := s.backupMgr.TryBeginRestoreOp(op, stack)
+	if busy == "" {
+		return "", true
+	}
+	if s.logger != nil {
+		s.logger.Printf("[WARN] [restore] %s of %s refused at its start — a %s operation runs (R-1256)", op, stack, busy)
+	}
+	msg, _ := s.restoreBusyMessage(busy, app)
+	return msg, false
+}
+
+// restoreBusyMessage is the household's sentence for a RestoreBusy reason ("" = not busy).
+func (s *Server) restoreBusyMessage(busy, app string) (string, bool) {
+	switch busy {
+	case "":
+		return "", false
+	case backup.RestoreBusyRestore:
 		subject := "Egy visszaállítási művelet"
-		if st.Stack != "" {
-			subject = "Egy visszaállítási művelet (" + st.Stack + ")"
+		if app != "" {
+			subject = "Egy visszaállítási művelet (" + app + ")"
 		}
 		return subject + " már fut, ezért most nem indítható újabb. Az állapotát ezen az oldalon " +
 			"követheted; amint befejeződik, újra indíthatsz visszaállítást.", true
-	}
-	if s.backupMgr.IsRunning() {
+	case backup.RestoreBusyBackup:
 		return "Egy mentési művelet már fut, ezért most nem indítható visszaállítás. Az állapotát " +
 			"ezen az oldalon követheted; amint befejeződik, újra indíthatsz visszaállítást.", true
-	}
-	// The stray-copy fix: a second-drive copy (rsync --delete) runs now, or one a previous controller process lost may
-	// still run — a restore beside it could lose the files it brings back (R-1180's other direction).
-	if app, blocked := s.backupMgr.Tier2CopyBlocksRestore(); blocked {
+	default:
+		// The stray-copy fix: a second-drive copy (rsync --delete) runs now, or one a previous controller process lost
+		// may still run — a restore beside it could lose the files it brings back (R-1180's other direction).
 		if s.logger != nil {
 			s.logger.Printf("[WARN] [restore] restore refused — a second-drive copy is running or may still run (app=%q)", app)
 		}
 		return "Éppen fut a második meghajtóra készülő mentési másolat, ezért most nem indítható visszaállítás. " +
 			"Néhány perc múlva próbáld újra.", true
 	}
-	return "", false
 }
 
 // backupsRestoreWizardHandler renders GET /backups/restore/app?name=<app> — the single entry the