Review fixes for 0.340.0: C5 a catch-up releases the whole-guest backup when its backup legs end (red-proved); C3 the R-1187 c refusal text fits an unhealthy app; C6 two comments; C8 README backup.volumes wording; unreleased

CHANGELOG.md

@@ -13,7 +13,15 @@
   meghajtóra készülő mentési másolat…"). The same night's review also moved the capped copy to `--mount` (a path the
   Docker daemon does not see is refused — exit 125, the in-process copy — instead of created empty, where `--delete`
   would empty the copy) and to the controller's image ID with `--pull never`; the copy container runs with `--no-healthcheck` (the image's
-  controller health check showed it as „starting" on the 9202 proof). Tests `TestStrayCopy_*` (backup, web,
+  controller health check showed it as „starting" on the 9202 proof).
+- **Review fixes (the same night's fresh review of everything in 0.340.0, felhom.eu `REPORT-night-2026-10-23.md`).**
+  C5: a catch-up stops holding the whole-guest backup when its backup legs end — the after-catch-up off-site proof and
+  integrity check (R-1189 d) no longer keep `Running()` true, which could push that day's whole-guest backup out of its
+  window on a big store; `TestC5_TheWholeGuestBackupDoesNotWaitForTheChecks`, red-proved. C3: the R-1187 (c) refusal's
+  text now fits an app that runs but is unhealthy or restarting (IsStackRunning counts only running/degraded) — „leállt,
+  vagy hibásan indul … ha hibásan fut, távolítsd el"; the import path for those states stays a row (R-1187). C6: two
+  main.go comments no longer say a box off at 05:30–06:00 always gets the proof made up (only with a missed backup leg).
+  C8: README — `backup.volumes` excluded leaves the app-data backups, not the `.fab` export or the whole-guest snapshot. Tests `TestStrayCopy_*` (backup, web,
   wiring; Docker-free seams), each red-proved; 9202 proof in felhom.eu `audits/night-2026-10-23/P1-straycopy/`.
 
 ## Unreleased — the second-drive copy no longer squeezes the apps' memory (R-1214), 2026-10-22 night

controller/README.md

@@ -1399,10 +1399,11 @@ retired in v0.126.0 when the moved blocks were legitimately rewritten onto the s
   contain no symlink (`tier2CappedPath`, `filepath.EvalSymlinks(p) == p`: a link could point the mount elsewhere). A
   docker start error (exit 125) falls back to plain rsync; any other failure is returned. Measured on 9202 (4 GB, 1.8 GB
   held, 6 GB copy): full stall 5.7 s → 0.37 s. Tests `r1214_capped_copy_test.go`.
-- **A template may leave a named volume out of every backup (unreleased, rides 0.340.0; R-49, operator D21 = B).**
+- **A template may leave a named volume out of the app-data backups (unreleased, rides 0.340.0; R-49, operator D21 = B).**
   `.felhom.yml` `backup.volumes: [{name: <volume>, class: excluded}]` — every named volume is dumped except
   `<project>_<name>` (`appbackup.ExcludedDockerVolumes` → `backup.Manager.dumpableVolumes`), so it leaves Tier 1, Tier 2
-  and off-site at once and the stale clean-up removes the old tar. Only `excluded` acts; an undeclared name is ignored
+  and off-site at once and the stale clean-up removes the old tar. NOT left out: the household's `.fab` export (it still
+  carries the volume) and the whole-guest PBS snapshot (it holds the guest's disk). Only `excluded` acts; an undeclared name is ignored
   and logged; a list that would exclude ALL of an app's named volumes is refused whole. First user: Immich's
   `immich_ml_cache` (model weights; Immich re-downloads them). Design: `07` §6.2.
 

controller/cmd/controller/main.go

@@ -1564,8 +1564,10 @@ func main() {
 					notifier.NotifyBackupCatchUp(missedAt.In(budapestLoc()).Format("15:04"))
 				},
 				// R-1189 (d): the off-site proof (05:30) and the integrity check (06:00) are daily jobs the scheduler
-				// never runs late; a box that is off at those hours comes back through this catch-up, so they run
-				// here — the proof picks one due snapshot, the integrity check runs only when it is due (max age).
+				// never runs late; a box that also MISSED a backup leg comes back through this catch-up, so they run
+				// here — the proof picks one due snapshot, the integrity check runs only when it is due (max age). A box
+				// that was on for the backup legs but off at 05:30–06:00 gets no catch-up: its integrity check catches
+				// up at the next 06:00 (due-ness); that day's proof is not made up (review 2026-10-22 night, C6).
 				After: func(ctx context.Context) {
 					runOffsiteProof(ctx, backupMgr, notifier, logger)
 					runOffsiteIntegrityCheck(ctx, backupMgr, notifier, logger, false)
@@ -1612,8 +1614,8 @@ func main() {
 		//
 		// DAILY JOB, WEEKLY BEHAVIOUR, and that is the point. It asks "is the last successful check
 		// older than the max age?" rather than "is it Sunday?", so a box that was switched off on its
-		// check day is checked the next day it is on AT 06:00 — or, if it is off at that hour, at its next
-		// catch-up (R-1189 d: the catch-up's After hook runs this check and the proof). R-341 is exactly the other shape: a dated check
+		// check day is checked the next day it is on AT 06:00 — or at its next catch-up, when it also missed a
+		// backup leg (R-1189 d: the catch-up's After hook runs this check and the proof). R-341 is exactly the other shape: a dated check
 		// quietly missed and never caught up. No `Weekly` primitive was added to the scheduler —
 		// due-ness is smaller, catches up, and is what R-86 already chose for restore-tests.
 		//
@@ -3137,7 +3139,7 @@ func (a *stackAdapter) GetDockerVolumes(name string) []string {
 	return backup.ResolveDockerVolumeNames(s.ComposePath)
 }
 
-// GetExcludedDockerVolumes (R-49, D21 = B): the named volumes the template excludes from every backup; an ignored
+// GetExcludedDockerVolumes (R-49, D21 = B): the named volumes the template excludes from every app-data backup (not the .fab export); an ignored
 // template entry is logged, never fatal (backup/volume_exclusion.go).
 func (a *stackAdapter) GetExcludedDockerVolumes(name string) []string {
 	ex, ignored := a.mgr.ExcludedDockerVolumes(name)

controller/internal/appexport/restore.go

@@ -362,11 +362,13 @@ func (e *Exporter) executeImport(req ImportRequest, job *Job) {
 	// NOT running used to „overwrite config in place" — its volumes were never removed, and `docker cp` only adds and
 	// overwrites, so the result was the old data MERGED with the bundle's (a stale SQLite -wal beside the bundle's .db
 	// is a corruption shape), reported as success. Only a running app's volumes are removed first. So this import is
-	// refused here, before anything is written; the household starts the app (or removes it) and imports again.
+	// refused here, before anything is written; the household starts the app (or removes it) and imports again. „Not
+	// running" is IsStackRunning's view: running or degraded only, so an unhealthy or restarting app is refused too —
+	// the text says so and names removal (review 2026-10-22 night, C3; the import path for those states is R-1187).
 	// Pinned by TestR1187c_AStoppedAppIsNotImportedOver.
 	if isDeployed && !isRunning {
 		e.logger.Printf("[WARN] Import: refused — %s is installed but not running; importing over it would merge the old data with the bundle's (R-1187 c)", manifest.AppName)
-		e.failJob(job, step, "Az alkalmazás telepítve van, de nem fut, ezért a csomagot nem töltjük vissza: a régi adatai összekeveredhetnének a csomagéval. Az alkalmazás érintetlen. Indítsd el (vagy távolítsd el), és próbáld újra.")
+		e.failJob(job, step, "Az alkalmazás telepítve van, de nem fut rendben (leállt, vagy hibásan indul), ezért a csomagot nem töltjük vissza: a régi adatai összekeveredhetnének a csomagéval. Az alkalmazás érintetlen. Ha leállt, indítsd el; ha hibásan fut, távolítsd el. Utána próbáld újra.")
 		return
 	}
 

controller/internal/nightchain/nightchain.go

@@ -340,6 +340,13 @@ func (c *CatchUp) run(ctx context.Context, why string) {
 		names = append(names, string(leg))
 	}
 	c.Ledger.markCatchUp(c.now())
+	// The backup legs are done: the whole-guest backup no longer waits for this catch-up (review 2026-10-22 night, C5).
+	// The checks below read only the off-site store, and on a big store they take long — held, they could push that
+	// day's whole-guest backup out of its window. `pending` stays set until run() returns (no second catch-up).
+	// Pinned by TestC5_TheWholeGuestBackupDoesNotWaitForTheChecks.
+	c.mu.Lock()
+	c.running = false
+	c.mu.Unlock()
 	if c.After != nil {
 		c.Logger.Printf("[INFO] [catch-up] running the after-catch-up checks (the off-site proof and the due-gated integrity check, R-1189 d)")
 		c.After(ctx)

controller/internal/nightchain/r1189d_test.go

@@ -43,3 +43,29 @@ func TestR1189d_TheChecksRunAfterTheCatchUpLegs(t *testing.T) {
 		t.Fatal("no catch-up ran, so the after-catch-up checks must not run")
 	}
 }
+
+// C5 (review 2026-10-22 night): while the after-catch-up checks run, the catch-up no longer reports Running() — the
+// scheduled whole-guest backup (quiesce.go, R-871) waits only for the backup legs, not for the off-site proof and the
+// integrity check, which take long on a big store. RED-PROOF: with the `c.running = false` before After removed,
+// this fails — seen failing 2026-10-22 night.
+func TestC5_TheWholeGuestBackupDoesNotWaitForTheChecks(t *testing.T) {
+	h := newHarness(t, at(1, 12, 0), at(5, 9, 0))
+	h.l.MarkEnded(LegDBDump, at(4, 2, 31))
+	h.l.MarkEnded(LegTier2, at(4, 3, 31))
+	h.l.MarkEnded(LegOffsite, at(4, 4, 20))
+	var mu sync.Mutex
+	runningDuringAfter := []bool{}
+	h.c.After = func(context.Context) {
+		r := h.c.Running()
+		mu.Lock()
+		runningDuringAfter = append(runningDuringAfter, r)
+		mu.Unlock()
+	}
+	h.c.Evaluate(context.Background(), "start")
+	h.wait(t)
+	mu.Lock()
+	defer mu.Unlock()
+	if len(runningDuringAfter) != 1 || runningDuringAfter[0] {
+		t.Fatalf("Running() during the after-catch-up checks = %v, want [false]", runningDuringAfter)
+	}
+}

controller/internal/stacks/metadata.go

@@ -597,7 +597,7 @@ func (m *Manager) ClassifiedBinds(name string) ([]appbackup.ClassifiedBind, bool
 	return appbackup.ClassifyBinds(meta.Backup, binds)
 }
 
-// ExcludedDockerVolumes returns the Docker names of the stack's named volumes its template excludes from every backup
+// ExcludedDockerVolumes returns the Docker names of the stack's named volumes its template excludes from every app-data backup
 // (R-49, D21 = B; appbackup.ExcludedDockerVolumes), and a sentence per template entry that was ignored.
 func (m *Manager) ExcludedDockerVolumes(name string) ([]string, []string) {
 	stack, ok := m.GetStack(name)