gate: the boot bind gate honours a customer's Stop (R-55, v0.157.0)

shouldRecreateOnBoot keyed on Deployed+drive-present alone. Deployed stays true across a Stop, so a drive-backed app the customer switched off was silently restarted on every guest reboot (proven live: immich). Requires len(Containers)>0 as well - R-52's existing-Exited vs absent distinction. A UI Stop is compose down and removes the containers; a guest that went down under a running app leaves them. Container STATE is still deliberately NOT a filter: that would miss a not-yet-restarted or stuck-Exited app, which is the bug the boot-id path exists to fix. Evidence sampled before any recreate - recreate's own StopStack erases it. Honoured Stops counted and logged separately from no-live-bind skips.
2026-07-21 14:53:11 +02:00
parent 83f20c8293
commit ac3790a11b
4 changed files with 200 additions and 41 deletions
@@ -1,6 +1,47 @@
 ## Changelog

-### v0.156.0 — a dead primary alerts (R-51); a boot orphan restarts itself (R-52) (2026-07-21)
+### v0.157.0 — the boot bind gate honours a customer's Stop (R-55) (2026-07-21)
+
+**Your Stop now means Stop across a guest reboot for drive-backed apps too** — the guarantee R-52
+already gave every other app. Found by STOP-1's R-52 leg on 2026-07-21, which was designed to prove
+the opposite: immich, stopped from the UI seconds earlier, came back running after the reboot.
+
+The boot bind gate (`internal/web/intermediary.go`) keyed its recreate on
+`Deployed && HDD_PATH && drive-present` alone. `Deployed` is a deploy-lifecycle flag — it stays true
+across a Stop — so the gate had no way to tell "the guest went down under this app" from "the
+customer switched this off", and it resurrected both. R-52 was never implicated: its own gate behaved
+exactly as specified (immich, at zero containers, was never a candidate for it). The gate simply
+reaches every drive-backed app first.
+
+**The fix is R-52's own predicate, translated.** `shouldRecreateOnBoot` now also requires
+`len(Stack.Containers) > 0` (from `docker ps -a`, so `Exited` containers count):
+
+- containers EXIST but are down → the guest went down under the app; docker's records survive the
+  reboot → boot orphan → recreate, as before.
+- ZERO containers → a UI Stop is `compose down`, which REMOVES the containers → deliberate → leave it.
+
+**What deliberately did NOT change: container STATE is still not a filter.** That is the original
+design's load-bearing part — a `State != stopped` filter misses an app that simply hasn't been
+auto-restarted yet after the boot, or is stuck `Exited` on a create-time bind failure with
+`RestartCount=0`. `hasContainers` is a different question ("does docker still have records of it")
+and, unlike liveness, it survives a reboot as a statement of intent. `TestShouldRecreateOnBoot` now
+pins both axes at once — they pull in opposite directions, which is the whole difficulty of this gate.
+
+- **Ordering trap, handled:** the evidence is sampled into the `bootStack` snapshot BEFORE any
+  recreate runs, because `recreate` calls `StopStack` (`compose down`) and so destroys the very
+  signal the decision needs.
+- **The drive-absent gate is not regressed.** Apps it stopped are also at zero containers, so this
+  path now skips them — correctly: they are recorded in `StoragePath.StoppedStacks` and restarted by
+  `ReconcileDriveGates`' `Return` branch, which runs on the same `driveGateLoop` tick.
+- **Honoured Stops are observable.** `leftStopped` is counted and logged separately from `skipped`
+  at INFO (`… left stopped — zero containers means the customer stopped them on purpose`).
+  Conflating them would have fired a WARN about a missing drive bind for an app behaving exactly as
+  asked, and a silent correct path is how an inert seam hides.
+- **Red-proof (run):** dropping `hasContainers` from the predicate makes
+  `TestRecreateDriveBackedApps_HonoursCustomerStop` fail with `recreated=[romm immich]` — the live
+  defect, by name.
+
+## v0.156.0 — a dead primary alerts (R-51); a boot orphan restarts itself (R-52) (2026-07-21)

 **No new agent coupling — MinAgent stays 0.90.0.** Two independent failures from the same live
 audit, both unattended-resilience holes: the box was broken and nobody was told, then the box could
@@ -1113,6 +1113,20 @@ not just those with HDD data. Non-HDD apps can configure destination, method, an
 >   covered; drives that never go live in the window are left to the drive-absent gate. `processGuestBootChange`
 >   also runs on every periodic `driveGateLoop` tick now (idempotent, boot-id gated) so a momentarily-unreachable
 >   agent right after a guest reboot no longer permanently strands recovery.
+>   **v0.157.0 — R-55, the gate now honours a customer's Stop.** Until this version the recreate keyed on
+>   `Deployed && HDD_PATH && drive-present` alone, so a drive-backed app the customer had deliberately
+>   Stopped was silently restarted on every guest reboot (proven live: immich, stopped from the UI seconds
+>   earlier, came back running). `shouldRecreateOnBoot` now also requires the app to still HAVE containers
+>   (`len(Stack.Containers) > 0`, from `docker ps -a`, so `Exited` ones count) — R-52's `existing-Exited vs
+>   absent` distinction (`bootrecon.isBootOrphan`) translated to this gate. A UI Stop is `compose down`,
+>   which REMOVES the containers; a guest that went down under a running app leaves them present. **State is
+>   still NOT a filter** — that part of the original design is load-bearing and unchanged; `hasContainers`
+>   answers a different question ("does docker still have records of it") which, unlike liveness, survives a
+>   reboot as a statement of intent. The evidence is sampled BEFORE any recreate, because recreate's own
+>   `StopStack` erases it. Apps stopped by the drive-absent gate are also at zero containers and are likewise
+>   left alone here — they are restored by `ReconcileDriveGates`' `Return` branch from
+>   `StoragePath.StoppedStacks`, on the same loop tick. Honoured Stops are logged at INFO (`left stopped …`),
+>   counted separately from the "no live bind" skips so an intended outcome never fires a WARN.
 >   **Agent-path prerequisite (also v0.71.0):** the whole drive gate needs `cfg.LocalAPI.Endpoint` (the
 >   per-guest agent local API). `bootstrap.MaybeIngest` now calls `ensureLocalAPI` on the already-configured
 >   path — merging `local_api` from `bootstrap.json` into an existing controller.yaml that lacks it (seeded
@@ -98,15 +98,39 @@ func agentWhere(registeredPath string) string {
 }

 // shouldRecreateOnBoot is the PURE decision for the boot-id recreate: on a fresh guest boot, recreate
-// EVERY deployed drive-backed app whose drive is present (BoundUnderParent) onto its (re-propagated)
-// drive. It is DETERMINISTIC — it depends ONLY on `app.yaml says should run` (deployed) + drive-present,
-// NOT on the app's current container state. The current state must NOT be a filter: a momentarily-stopped
-// app on a fresh reboot (docker hasn't auto-restarted it yet) would otherwise be MISSED — the exact bug
-// the boot-id path replaces (the old container-uptime sample, and a State!=stopped filter, both miss it).
-// (Tradeoff: a UI-stopped drive-backed app is brought back on a guest reboot — `deployed` is the only
-// "should run" signal app.yaml carries; the gate manages drive-backed app lifecycle otherwise.)
-func shouldRecreateOnBoot(deployed bool, hdd string, presentStable map[string]bool) bool {
-	return deployed && hdd != "" && strings.HasPrefix(hdd, StableParentDir+"/") && presentStable[hdd]
+// a deployed drive-backed app whose drive is present (BoundUnderParent) onto its (re-propagated) drive.
+//
+// It is DETERMINISTIC and it still does NOT filter on the app's current container STATE. That remains
+// load-bearing: a momentarily-stopped app on a fresh reboot (docker hasn't auto-restarted it yet, or
+// its create-time bind failed with RestartCount=0) must be recreated, and both the old
+// container-uptime sample and a `State != stopped` filter MISS exactly that case. Do not reintroduce
+// a state filter here.
+//
+// R-55: it DOES filter on whether the app still HAS containers, which is a different question and the
+// one that tells the truth about intent. This is R-52's `existing-Exited vs absent` distinction
+// (bootrecon.isBootOrphan), translated to this gate:
+//
+//   - containers EXIST but are down  → the guest went down under the app; docker's own records survive
+//     the reboot, so this is a boot orphan → recreate.
+//   - ZERO containers                → a UI Stop is `compose down`, which REMOVES the containers.
+//     Nothing else in the controller leaves a deployed app at zero containers. → the customer stopped
+//     this on purpose → LEAVE IT ALONE.
+//
+// `deployed` cannot answer this: it is a deploy-lifecycle flag and stays true across a Stop. Before
+// R-55 the gate had no other signal and therefore silently undid a customer's Stop on every guest
+// reboot — including when apps were stopped deliberately to free resources for others, which is
+// precisely when resurrecting them is most harmful.
+//
+// The evidence is read from the snapshot taken BEFORE any recreate runs, because `recreate` itself
+// calls StopStack (`compose down`) and so destroys it.
+//
+// NOTE on the drive-absent gate: apps it stopped are also at zero containers, so they are skipped
+// here too. That is correct — they are recorded in StoragePath.StoppedStacks and restarted by
+// ReconcileDriveGates' `Return` branch, which runs on the same loop tick. Their recovery is that
+// path's job, not this one's.
+func shouldRecreateOnBoot(deployed bool, hdd string, presentStable map[string]bool, hasContainers bool) bool {
+	return deployed && hdd != "" && strings.HasPrefix(hdd, StableParentDir+"/") &&
+		presentStable[hdd] && hasContainers
 }

 // defaultPromotionTarget decides M1 (never leave zero default). If the path being decommissioned is NOT
@@ -383,7 +407,11 @@ func (s *Server) processGuestBootChange() {
 		if cfg == nil {
 			continue
 		}
-		bootStacks = append(bootStacks, bootStack{name: st.Name, deployed: cfg.Deployed, hdd: cfg.Env["HDD_PATH"], state: string(st.State)})
+		// hasContainers is sampled HERE, before any recreate — recreate's StopStack destroys it (R-55).
+		bootStacks = append(bootStacks, bootStack{
+			name: st.Name, deployed: cfg.Deployed, hdd: cfg.Env["HDD_PATH"], state: string(st.State),
+			hasContainers: len(st.Containers) > 0,
+		})
 	}
 	recreate := func(bs bootStack) {
 		s.logger.Printf("[INFO] [gate] boot %s: live bind confirmed — recreating drive-backed app %s (state=%s) onto %s", resp.GuestBootID, bs.name, bs.state, bs.hdd)
@@ -396,10 +424,15 @@ func (s *Server) processGuestBootChange() {
 		s.logger.Printf("[INFO] [gate] boot %s: re-syncing FileBrowser mounts against the live binds", resp.GuestBootID)
 		go s.SyncFileBrowserMounts()
 	}
-	_, skipped := recreateDriveBackedApps(bootStacks, presentStable, recreate, syncFB)
+	_, skipped, leftStopped := recreateDriveBackedApps(bootStacks, presentStable, recreate, syncFB)
 	if skipped > 0 {
 		s.logger.Printf("[WARN] [gate] boot %s: %d drive-backed app(s) had no live bind within %s — leaving to the drive gate", resp.GuestBootID, skipped, bootBindWait)
 	}
+	if leftStopped > 0 {
+		// INFO, not WARN: this is the gate working as intended (R-55). Make the honoured path
+		// observable — a silent correct path is how an inert seam hides.
+		s.logger.Printf("[INFO] [gate] boot %s: %d drive-backed app(s) left stopped — zero containers means the customer stopped them on purpose", resp.GuestBootID, leftStopped)
+	}
 	if serr := s.settings.SetLastGuestBootID(resp.GuestBootID); serr != nil {
 		s.logger.Printf("[WARN] [gate] persist boot-id: %v", serr)
 	}
@@ -411,6 +444,10 @@ type bootStack struct {
 	deployed bool
 	hdd      string
 	state    string
+	// hasContainers is len(Stack.Containers) > 0, from `docker ps -a` — so Exited containers COUNT.
+	// R-55's running-at-shutdown signal: a UI Stop is `compose down` and leaves zero. MUST be sampled
+	// before any recreate runs, since recreate's StopStack erases it.
+	hasContainers bool
 }

 // recreateDriveBackedApps recreates every deployed drive-backed app whose drive bind is live, then
@@ -419,12 +456,21 @@ type bootStack struct {
 // ran once pollLiveBinds confirmed the live binds), so FileBrowser's mounts reflect the now-live drives
 // instead of going stale (the gap a host/guest reboot left before this fix). syncFB is always called so
 // FileBrowser reflects the current bind state even if no app needed recreating. Pure (ops injected).
-func recreateDriveBackedApps(stacks []bootStack, presentStable map[string]bool, recreate func(bootStack), syncFB func()) (recreated, skipped int) {
+// R-55: `leftStopped` counts drive-backed apps deliberately NOT touched because they have zero
+// containers (a customer Stop). It is reported separately from `skipped` — conflating the two would
+// make an honoured Stop look like the "bind never went live" failure and fire a WARN for healthy,
+// intended behaviour.
+func recreateDriveBackedApps(stacks []bootStack, presentStable map[string]bool, recreate func(bootStack), syncFB func()) (recreated, skipped, leftStopped int) {
 	for _, bs := range stacks {
-		if !shouldRecreateOnBoot(bs.deployed, bs.hdd, presentStable) {
+		if !shouldRecreateOnBoot(bs.deployed, bs.hdd, presentStable, bs.hasContainers) {
 			if bs.deployed && strings.HasPrefix(bs.hdd, StableParentDir+"/") {
+				switch {
+				case presentStable[bs.hdd] && !bs.hasContainers:
+					leftStopped++ // drive IS live; the app is at zero containers → stopped on purpose
+				default:
 					skipped++ // a deployed drive-backed app whose bind never went live → gate's job
 				}
+			}
 			continue
 		}
 		recreate(bs)
@@ -21,34 +21,92 @@ func TestAgentWhere(t *testing.T) {
 	}
 }

-// TestShouldRecreateOnBoot pins the DETERMINISTIC boot-id recreate decision: recreate EVERY deployed
-// drive-backed present app, independent of its current container state.
+// TestShouldRecreateOnBoot pins the boot-id recreate decision on BOTH axes it must get right at once:
+// recreate every deployed drive-backed present app that still HAS containers, independent of what
+// STATE those containers are in.
 //
-// COMPANION GUARD: the pre-fix logic (the old `stackStartedRecently`, and even a `State!=stopped` filter)
-// MISSED an app that was Stopped/Exited at the one-shot instant — docker hadn't auto-restarted it yet
-// after the boot. The "exited" and "stopped" cases below are `true` here: a state-filtered impl returns
-// false for them and the app stays down (exactly what happened live: 5 apps exited after a host reboot).
+// COMPANION GUARD 1 (state must NOT filter): the pre-fix logic (the old `stackStartedRecently`, and
+// even a `State!=stopped` filter) MISSED an app that was Stopped/Exited at the one-shot instant —
+// docker hadn't auto-restarted it yet after the boot. The exited/stopped cases below are `true`: a
+// state-filtered impl returns false for them and the app stays down (exactly what happened live: 5
+// apps exited after a host reboot).
+//
+// COMPANION GUARD 2 (R-55: containers MUST filter): an impl that ignores `hasContainers` resurrects
+// an app the customer deliberately stopped, on every guest reboot. The zero-container cases below are
+// `false`; the pre-R-55 impl returns true for them (proven live: immich, stopped from the UI seconds
+// earlier, came back running).
+//
+// The two guards pull in opposite directions on purpose — that is the whole difficulty of this gate.
+// `state` and `hasContainers` are DIFFERENT questions: "is it up right now" vs "does docker still
+// have records of it", and only the second survives a reboot as a statement of intent.
 func TestShouldRecreateOnBoot(t *testing.T) {
-	present := map[string]bool{"/mnt/felhom-drives/felhom-flash": true}
+	const flash = "/mnt/felhom-drives/felhom-flash"
+	present := map[string]bool{flash: true}
 	cases := []struct {
 		name          string
 		deployed      bool
 		hdd           string
+		hasContainers bool
 		want          bool
 	}{
-		{"deployed+present (recreate regardless of state)", true, "/mnt/felhom-drives/felhom-flash", true},
-		{"drive absent (gate handles)", true, "/mnt/felhom-drives/felhom-usb", false},
-		{"SSD path never", true, "/mnt/sys_drive/felhom-data", false},
-		{"app.yaml not deployed", false, "/mnt/felhom-drives/felhom-flash", false},
-		{"no HDD_PATH (SSD-resident)", true, "", false},
+		// --- has containers: recreate regardless of what state they are in (guard 1) ---
+		{"deployed+present+containers (running)", true, flash, true, true},
+		{"deployed+present+containers (exited after boot)", true, flash, true, true},
+		{"deployed+present+containers (stuck create-time failure)", true, flash, true, true},
+
+		// --- R-55: zero containers == `compose down` == the customer's Stop (guard 2) ---
+		{"R-55 customer-stopped drive app (zero containers)", true, flash, false, false},
+		{"R-55 gate-stopped app (zero containers; Return branch owns it)", true, flash, false, false},
+
+		// --- the pre-existing axes, unchanged ---
+		{"drive absent (gate handles)", true, "/mnt/felhom-drives/felhom-usb", true, false},
+		{"SSD path never", true, "/mnt/sys_drive/felhom-data", true, false},
+		{"app.yaml not deployed", false, flash, true, false},
+		{"no HDD_PATH (SSD-resident)", true, "", true, false},
 	}
 	for _, c := range cases {
-		if got := shouldRecreateOnBoot(c.deployed, c.hdd, present); got != c.want {
+		if got := shouldRecreateOnBoot(c.deployed, c.hdd, present, c.hasContainers); got != c.want {
 			t.Errorf("%s: shouldRecreateOnBoot = %v, want %v", c.name, got, c.want)
 		}
 	}
 }

+// TestRecreateDriveBackedApps_HonoursCustomerStop is the R-55 regression at the driver level, and it
+// separates the two reasons an app is not recreated — a conflated counter would fire a WARN about a
+// missing drive bind for an app that is stopped exactly as the customer asked.
+//
+// COMPANION GUARD: drop `hasContainers` from the predicate and `immich` is recreated — the exact live
+// defect (REPORT §4b). Drop the counter split and `leftStopped` reads 0 while `skipped` reads 2.
+func TestRecreateDriveBackedApps_HonoursCustomerStop(t *testing.T) {
+	const flash = "/mnt/felhom-drives/felhom-flash"
+	present := map[string]bool{flash: true}
+	stacks := []bootStack{
+		// boot orphan: containers exist but are down → MUST be recreated.
+		{name: "romm", deployed: true, hdd: flash, state: "exited", hasContainers: true},
+		// customer pressed Stop → compose down → zero containers → MUST be left alone.
+		{name: "immich", deployed: true, hdd: flash, state: "stopped", hasContainers: false},
+		// drive-backed but the bind never went live → the drive gate's job, counted separately.
+		{name: "stranded", deployed: true, hdd: "/mnt/felhom-drives/felhom-usb", hasContainers: true},
+	}
+	var recreated []string
+	n, skipped, leftStopped := recreateDriveBackedApps(stacks, present,
+		func(bs bootStack) { recreated = append(recreated, bs.name) }, func() {})
+
+	if len(recreated) != 1 || recreated[0] != "romm" {
+		t.Fatalf("recreated=%v, want exactly [romm] — a customer-stopped app must never be restarted", recreated)
+	}
+	for _, name := range recreated {
+		if name == "immich" {
+			t.Fatal("immich was stopped from the UI and was recreated anyway — R-55 regression")
+		}
+	}
+	if n != 1 || skipped != 1 || leftStopped != 1 {
+		t.Fatalf("recreated=%d skipped=%d leftStopped=%d, want 1/1/1 "+
+			"(skipped = no live bind; leftStopped = honoured customer Stop — these must not be conflated)",
+			n, skipped, leftStopped)
+	}
+}
+
 // TestDefaultPromotionTarget pins M1 (never leave zero default).
 //
 // COMPANION GUARD: the pre-fix decommission blanked the default and promoted nothing — equivalent to this
@@ -100,7 +158,7 @@ func TestPollLiveBinds_WaitsForLateBind(t *testing.T) {
 	if nowT < goLive { // proves it WAITED through the rebind window (a single sample would return at t=0)
 		t.Fatalf("poll returned at t=%s before the bind went live at %s — it did not wait (regression)", nowT, goLive)
 	}
-	if !shouldRecreateOnBoot(true, flash, live) {
+	if !shouldRecreateOnBoot(true, flash, live, true) {
 		t.Fatalf("with the live bind present, the drive-backed app MUST be recreated")
 	}
 }
@@ -112,7 +170,7 @@ func TestSingleEarlySample_MissesLateBind_Companion(t *testing.T) {
 	flash := "/mnt/felhom-drives/felhom-flash"
 	bindLiveAtBoot := func(string) bool { return false } // not yet live at the boot instant
 	oldPresent := map[string]bool{flash: bindLiveAtBoot(flash)}
-	if shouldRecreateOnBoot(true, flash, oldPresent) {
+	if shouldRecreateOnBoot(true, flash, oldPresent, true) {
 		t.Fatalf("companion: a single early sample reads the not-yet-live bind as absent and must MISS it")
 	}
 }
@@ -134,7 +192,7 @@ func TestPollLiveBinds_TimeoutLeavesAbsent(t *testing.T) {
 	if nowT < bootBindWait {
 		t.Fatalf("poll must run to the deadline for an absent drive, t=%s", nowT)
 	}
-	if shouldRecreateOnBoot(true, usb, live) {
+	if shouldRecreateOnBoot(true, usb, live, true) {
 		t.Fatalf("an absent drive's app must NOT be recreated here (the gate owns drive-absent)")
 	}
 }
@@ -146,18 +204,18 @@ func TestRecreateDriveBackedApps_SyncsFileBrowserAfterRecreate(t *testing.T) {
 	flash := "/mnt/felhom-drives/felhom-flash"
 	present := map[string]bool{flash: true}
 	stacks := []bootStack{
-		{name: "romm", deployed: true, hdd: flash},                                  // drive-backed, live → recreate
-		{name: "actualbudget", deployed: true, hdd: "/mnt/sys_drive/felhom-data"},    // SSD → not recreated
-		{name: "stranded", deployed: true, hdd: "/mnt/felhom-drives/felhom-usb"},     // drive-backed, bind NOT live → skipped
+		{name: "romm", deployed: true, hdd: flash, hasContainers: true},                               // drive-backed, live → recreate
+		{name: "actualbudget", deployed: true, hdd: "/mnt/sys_drive/felhom-data", hasContainers: true}, // SSD → not recreated
+		{name: "stranded", deployed: true, hdd: "/mnt/felhom-drives/felhom-usb", hasContainers: true},  // drive-backed, bind NOT live → skipped
 	}
 	var seq []string
 	recreate := func(bs bootStack) { seq = append(seq, "recreate:"+bs.name) }
 	syncFB := func() { seq = append(seq, "syncFB") }

-	recreated, skipped := recreateDriveBackedApps(stacks, present, recreate, syncFB)
+	recreated, skipped, leftStopped := recreateDriveBackedApps(stacks, present, recreate, syncFB)

-	if recreated != 1 || skipped != 1 {
-		t.Fatalf("recreated=%d skipped=%d, want 1/1", recreated, skipped)
+	if recreated != 1 || skipped != 1 || leftStopped != 0 {
+		t.Fatalf("recreated=%d skipped=%d leftStopped=%d, want 1/1/0", recreated, skipped, leftStopped)
 	}
 	// FileBrowser sync MUST be invoked, and AFTER every recreate.
 	if len(seq) != 2 || seq[0] != "recreate:romm" || seq[len(seq)-1] != "syncFB" {