v0.278.0: a hold left by an earlier install no longer holds the new one (R-704)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS

controller/README.md

@@ -1916,6 +1916,8 @@ that folder is never a dead end, and an install never runs into it silently (R-6
 - **Where it lives.** `<drive>/kept/` is beside `appdata/` and `userdata/`, inside neither: no app bind, FileBrowser
   userdata source, Samba share or backup leg reads it. It is in `ProtectedHDDPaths`.
 - **The drive-full warning** ends with the kept folders on that drive and their sizes (`fillwatch.SetExtra`).
+- **R-704 (v0.278.0).** A new install (plain or "use my kept data") drops an update or crash-loop hold left by an EARLIER
+  install of the app, and a removal clears both kinds; a restore hold (R-379) stays operator-cleared.
 - **R-690 (fixed here).** The removed-app restore (R-487) never found a unit on a DATA drive — it asked
   `GetStackComposePath`, true for every catalog app — and restored with no env. It now asks `isStackDeployed`.
 - `.felhom.yml` **`after_load: {service, user, command: [...]}`** — one command run with `docker compose exec -T`

controller/internal/api/kept_install.go

@@ -95,6 +95,7 @@ func (r *Router) keptDataAtInstall(w http.ResponseWriter, req *http.Request, nam
 			src = "off-site snapshot " + cp.c.SnapshotID
 		}
 		r.logger.Printf("[INFO] [api] Deploy %s: USE MY KEPT DATA — loading from %s (tier %d, %s) under the kept files %v", name, src, cp.c.Tier, cp.c.Time.UTC().Format(time.RFC3339), old)
+		r.dropLeftoverHold(name, "use-kept-data") // R-704: this is a new install of an app that is not installed
 		r.startKeptLoad(name, hdd, cp.c, lang, nil)
 		writeJSON(w, http.StatusAccepted, apiResponse{OK: true, Message: r.msgLang(lang, "kept.load.started", display)})
 		return true

controller/internal/api/kept_install_test.go

@@ -3,12 +3,16 @@ package api
 import (
 	"context"
 	"encoding/json"
+	"go/ast"
+	"go/parser"
+	"go/token"
 	"io"
 	"log"
 	"net/http"
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -151,3 +155,135 @@ func TestR691_InstallChoiceNamesTheOffsiteCopy(t *testing.T) {
 		t.Fatal("something was installed")
 	}
 }
+
+// R-704 (v0.278.0) — a hold an EARLIER install left under the app's name is dropped when a new install
+// begins; the consequence measured on demo-hp was a fresh nextcloud whose backups skipped its volumes and
+// unit because of a 2026-09-13 update hold. Driven through the real „use my kept data" path (the off-site
+// copy, restic and the unit restore behind their seams — no Docker).
+// COMPANION RED-PROOF: remove the dropLeftoverHold call from keptDataAtInstall → the hold is still there
+// after the 202 and the first assertion fails; remove it from deployStack → the AST check fails.
+func TestR704_AFreshInstallDropsALeftoverHold(t *testing.T) {
+	dir := t.TempDir()
+	drive := filepath.Join(dir, "drive")
+	cfg := &config.Config{}
+	cfg.Paths.StacksDir = filepath.Join(dir, "stacks")
+	cfg.Paths.DataDir = filepath.Join(dir, "data")
+	cfg.Stacks.ComposeCommand = "docker compose"
+	app := filepath.Join(cfg.Paths.StacksDir, "cloudapp")
+	for _, d := range []string{app, filepath.Join(drive, "appdata/cloudapp"), cfg.Paths.DataDir} {
+		if err := os.MkdirAll(d, 0o755); err != nil {
+			t.Fatal(err)
+		}
+	}
+	_ = os.WriteFile(filepath.Join(app, "docker-compose.yml"), []byte("services:\n  cloudapp:\n    image: busybox:1\n    volumes:\n      - ${HDD_PATH}/appdata/cloudapp:/data\n"), 0o644)
+	_ = os.WriteFile(filepath.Join(app, ".felhom.yml"), []byte("display_name: Cloud App\n"), 0o644)
+	_ = os.WriteFile(filepath.Join(drive, "appdata/cloudapp/old.txt"), []byte("old"), 0o644)
+	m, err := stacks.NewManager(cfg, log.New(io.Discard, "", 0))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := m.ScanStacks(); err != nil {
+		t.Fatal(err)
+	}
+	sett, err := settings.Load(filepath.Join(cfg.Paths.DataDir, "settings.json"), log.New(io.Discard, "", 0))
+	if err != nil {
+		t.Fatal(err)
+	}
+	_ = sett.SetOffboxTarget(&settings.OffboxTarget{Enabled: true, Host: "nas.local", Port: 22, User: "felhom",
+		RepoPath: "/srv/repo", Schedule: "daily", EscrowState: "escrowed"})
+	if err := sett.AddStoragePath(settings.StoragePath{Path: drive, Label: "HDD", Schedulable: true}); err != nil {
+		t.Fatal(err)
+	}
+	bm := backup.NewManager(cfg, sett, log.New(io.Discard, "", 0))
+	if err := bm.WriteOffboxSecrets("KEY", "nas.local ssh-ed25519 AAAA"); err != nil {
+		t.Fatal(err)
+	}
+	bm.SetOffboxFreeFn(func(string) int64 { return 100 << 30 })
+	unitPath := drive + "/backups/primary/cloudapp"
+	bm.SetOffboxRunner(func(_ context.Context, _ []string, args ...string) ([]byte, error) {
+		var target string
+		isRestore := false
+		for i, a := range args {
+			if a == "snapshots" {
+				return json.Marshal([]map[string]interface{}{{"short_id": "ab12cd34", "time": time.Now().Add(-time.Hour),
+					"tags": []string{"cloudapp"}, "paths": []string{unitPath}}})
+			}
+			if a == "restore" {
+				isRestore = true
+			}
+			if a == "--target" && i+1 < len(args) {
+				target = args[i+1]
+			}
+		}
+		if isRestore && target != "" {
+			u := filepath.Join(target, strings.TrimPrefix(unitPath, "/"))
+			_ = os.MkdirAll(filepath.Join(u, "compose"), 0o755)
+			_ = os.MkdirAll(filepath.Join(u, "db-dumps"), 0o755)
+			_ = os.WriteFile(filepath.Join(u, "db-dumps", "cloudapp.sql"), []byte("x"), 0o644)
+			_ = os.WriteFile(filepath.Join(u, "compose", "docker-compose.yml"), []byte("services:\n  cloudapp:\n    image: busybox:1\n"), 0o644)
+			_ = os.WriteFile(filepath.Join(u, "compose", "app.yaml"), []byte("env:\n  HDD_PATH: "+drive+"\n"), 0o600)
+			man, _ := json.Marshal(map[string]interface{}{"schema_version": 2, "app_name": "cloudapp", "db_dumps": []string{"cloudapp.sql"},
+				"data": map[string]interface{}{"at": time.Now().UTC().Format(time.RFC3339), "image_pins": []string{"busybox:1"}}})
+			_ = os.WriteFile(filepath.Join(u, "manifest.json"), man, 0o644)
+		}
+		return nil, nil
+	})
+	done := make(chan struct{}, 1)
+	bm.SetKeptUnitRestoreFn(func(app, unitDir string) (backup.UnitRestoreResult, error) {
+		done <- struct{}{}
+		return backup.UnitRestoreResult{}, nil
+	})
+	r := &Router{stackMgr: m, backupMgr: bm, sett: sett, logger: log.New(io.Discard, "", 0)}
+
+	// A leftover update hold from an earlier install (the demo-hp shape): the new install drops it.
+	_ = sett.SetRestoreHold(settings.RestoreHold{Stack: "cloudapp", At: "2026-09-13T19:56:32Z", Reason: settings.HoldReasonUpdateFailed})
+	w := httptest.NewRecorder()
+	req := httptest.NewRequest(http.MethodPost, "/api/stacks/cloudapp/deploy?lang=en", nil)
+	if !r.keptDataAtInstall(w, req, "cloudapp", drive, "use") || w.Code != http.StatusAccepted {
+		t.Fatalf("use: code %d body %s", w.Code, w.Body.String())
+	}
+	if h, held := sett.GetRestoreHold("cloudapp"); held {
+		t.Fatalf("the earlier install's hold survived the new install: %+v", h)
+	}
+	select {
+	case <-done:
+	case <-time.After(10 * time.Second):
+		t.Fatal("the load never reached the unit restore")
+	}
+
+	// A restore hold (R-379) is NEVER dropped by an install.
+	_ = sett.SetRestoreHold(settings.RestoreHold{Stack: "cloudapp", At: "2026-09-13T19:56:32Z", Reason: settings.HoldReasonRestoreFailed})
+	r.dropLeftoverHold("cloudapp", "install")
+	if _, held := sett.GetRestoreHold("cloudapp"); !held {
+		t.Fatal("an install dropped an R-379 restore hold")
+	}
+	_, _ = sett.ClearRestoreHold("cloudapp")
+
+	// The plain install path drops it too — before DeployStack (AST: the call precedes it in deployStack).
+	fset := token.NewFileSet()
+	f, err := parser.ParseFile(fset, "router.go", nil, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var dropAt, deployAt token.Pos
+	for _, d := range f.Decls {
+		fn, ok := d.(*ast.FuncDecl)
+		if !ok || fn.Name.Name != "deployStack" || fn.Body == nil {
+			continue
+		}
+		ast.Inspect(fn.Body, func(n ast.Node) bool {
+			if s, ok := n.(*ast.SelectorExpr); ok {
+				switch s.Sel.Name {
+				case "dropLeftoverHold":
+					dropAt = s.Pos()
+				case "DeployStack":
+					deployAt = s.Pos()
+				}
+			}
+			return true
+		})
+	}
+	if dropAt == token.NoPos || deployAt == token.NoPos || dropAt > deployAt {
+		t.Fatalf("deployStack must call dropLeftoverHold before DeployStack (drop=%v deploy=%v)", dropAt, deployAt)
+	}
+}

controller/internal/api/router.go

@@ -496,6 +496,9 @@ func (r *Router) deployStack(w http.ResponseWriter, req *http.Request, name stri
 		return
 	}
 
+	// R-704 (v0.278.0): a fresh install drops a hold an EARLIER install left under this name.
+	r.dropLeftoverHold(name, "install")
+
 	deployReq := stacks.DeployRequest{
 		StackName: name,
 		Values:    body.Values,
@@ -905,6 +908,27 @@ func (r *Router) getStackBackupData(w http.ResponseWriter, req *http.Request, na
 	writeJSON(w, http.StatusOK, apiResponse{OK: true, Data: resp})
 }
 
+// dropLeftoverHold (R-704, v0.278.0) clears the update or crash-loop hold of an app that is NOT installed,
+// before a new install of it begins (a plain install, or „use my kept data"). Such a hold can only have
+// been left by an earlier install: before v0.242.0 a removal kept update holds, and before v0.278.0 it
+// kept crash-loop stops — measured 2026-09-28: a nextcloud update hold from 2026-09-13 on demo-hp made
+// the backup skip the FRESH install's volumes and unit, so its off-site copy held no data; on 9202 a
+// calcom crash-loop stop refused the fresh install's update. A restore hold (R-379) is never touched.
+// Pinned by TestR704_AFreshInstallDropsALeftoverHold.
+func (r *Router) dropLeftoverHold(name, why string) {
+	if r.sett == nil {
+		return
+	}
+	if st, ok := r.stackMgr.GetStack(name); ok && (st.Deployed || st.Deploying) {
+		return
+	}
+	if cleared, err := r.sett.ClearUpdateHold(name); err != nil {
+		r.logger.Printf("[WARN] [api] %s %s: the hold an earlier install left could not be cleared: %v", why, name, err)
+	} else if cleared {
+		r.logger.Printf("[INFO] [api] %s %s: a hold left by an EARLIER install of this app is dropped (R-704)", why, name)
+	}
+}
+
 func (r *Router) removeStack(w http.ResponseWriter, req *http.Request, name string) {
 	if name == "" {
 		writeJSON(w, http.StatusBadRequest, apiResponse{OK: false, Error: "invalid stack name"})

controller/internal/backup/kept_load.go

@@ -319,7 +319,7 @@ func (m *Manager) LoadKeptApp(app, unitDir string, okMsg, failMsg func(err error
 	m.BeginRestoreOp("restore", app)
 	go func() {
 		start := time.Now()
-		res, err := m.RestoreFromRecoveryUnitAt(app, unitDir)
+		res, err := m.keptUnitRestore()(app, unitDir)
 		if err != nil {
 			m.logger.Printf("[ERROR] [backup] kept load %s from %s FAILED after %s: %v", app, unitDir, time.Since(start).Round(time.Second), err)
 			m.EndRestoreOp(false, failMsg(err))

controller/internal/settings/r704_hold_test.go

@@ -0,0 +1,38 @@
+package settings
+
+import (
+	"io"
+	"log"
+	"path/filepath"
+	"testing"
+)
+
+// R-704 (v0.278.0) — ClearUpdateHold clears the holds that belong to an INSTALL (update, crash-loop stop) and
+// never a restore hold (R-379), which stays operator-cleared.
+// COMPANION RED-PROOF: the pre-fix predicate (`h.Reason != HoldReasonUpdateFailed`) → the crash-loop case fails.
+func TestR704_ClearUpdateHoldClearsTheInstallsHoldsOnly(t *testing.T) {
+	s, err := Load(filepath.Join(t.TempDir(), "settings.json"), log.New(io.Discard, "", 0))
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, c := range []struct {
+		reason string
+		want   bool
+	}{{HoldReasonUpdateFailed, true}, {HoldReasonUnhealthyStop, true}, {HoldReasonRestoreFailed, false}} {
+		if err := s.SetRestoreHold(RestoreHold{Stack: "app", At: "2026-09-13T19:56:32Z", Reason: c.reason}); err != nil {
+			t.Fatal(err)
+		}
+		cleared, err := s.ClearUpdateHold("app")
+		if err != nil {
+			t.Fatal(err)
+		}
+		_, still := s.GetRestoreHold("app")
+		if cleared != c.want || still == c.want {
+			t.Fatalf("reason %q: cleared=%v still-held=%v, want cleared=%v", c.reason, cleared, still, c.want)
+		}
+		_, _ = s.ClearRestoreHold("app")
+	}
+	if cleared, _ := s.ClearUpdateHold("none"); cleared {
+		t.Fatal("clearing an app with no hold reported a clear")
+	}
+}

controller/internal/settings/settings.go

@@ -1917,19 +1917,22 @@ func (s *Settings) ClearRestoreHold(stack string) (bool, error) {
 	return true, s.save()
 }
 
-// ClearUpdateHold (R-491, v0.242.0) removes an app's hold ONLY when it is an update hold. An R-379
-// restore hold stays: that one means a database was left in an unknown state and is cleared by the
-// operator (`-clear-restore-hold`), never by a removal. Returns whether an update hold was cleared.
+// ClearUpdateHold (R-491, v0.242.0) removes an app's hold when it belongs to the app's INSTALL: an update
+// hold, and since v0.278.0 (R-704) the box's crash-loop/OOM stop (HoldReasonUnhealthyStop) — both name an
+// install that a removal ends, and the name is all a later install shares with it. An R-379 restore hold
+// stays: that one means a database was left in an unknown state and is cleared by the operator
+// (`-clear-restore-hold`), never by a removal or an install. Returns whether a hold was cleared.
+// Pinned by TestR704_ClearUpdateHoldClearsTheInstallsHoldsOnly.
 func (s *Settings) ClearUpdateHold(stack string) (bool, error) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 	h, ok := s.RestoreHolds[stack]
-	if !ok || h.Reason != HoldReasonUpdateFailed {
+	if !ok || (h.Reason != HoldReasonUpdateFailed && h.Reason != HoldReasonUnhealthyStop) {
 		return false, nil
 	}
 	delete(s.RestoreHolds, stack)
 	if s.log != nil {
-		s.log.Printf("[INFO] [settings] update hold CLEARED for %s (the app was removed)", stack)
+		s.log.Printf("[INFO] [settings] %s hold CLEARED for %s (set %s; its install is gone)", h.Reason, stack, h.At)
 	}
 	return true, s.save()
 }