From c23a0f6d2d6f4494933a08000d8e8e14267bcc29 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Fri, 24 Jul 2026 10:50:14 +0200 Subject: [PATCH] v0.164.0: deliberately stopped apps no longer alarm (banner + email) A UI stop (Leallitas -> compose down -> StateStopped) is the user's own action, not a fault, and must not raise the deadapp banner OR the app_start_failed event. Filter at the single fix-3 derivation point: extract scanDeployedAppRunStates's pure core to classifyRunStates and change the down predicate to IsDownState(st.State) && st.State != StateStopped. Suppresses StateStopped from both the banner dead-list and the notifier Down-set at once. Rests on two invariants (recorded at the seam, README, CONTEXT): I1 StopStack = compose down => zero containers => StateStopped I2 P2 census: all catalog services unless-stopped => faults never rest at stopped (they surface as exited/degraded). IsDownState unchanged; out-of-band 'compose stop' (containers remain -> exited) still alerts. Tests +4 (notify 3->4, main 4->7), both red-proofs verified. No template/funcmap/notifier/counter/copy change. --- CHANGELOG.md | 36 +++++ CONTEXT.md | 20 ++- REUSE.md | 1 + controller/README.md | 20 ++- .../cmd/controller/classify_runstates_test.go | 123 ++++++++++++++++++ controller/cmd/controller/main.go | 27 +++- controller/internal/notify/deadapp_test.go | 36 +++++ 7 files changed, 257 insertions(+), 6 deletions(-) create mode 100644 controller/cmd/controller/classify_runstates_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f982ee..2c3f6e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,41 @@ ## Changelog +### v0.164.0 — Deliberately stopped apps no longer alarm (banner + email) (2026-07-24) + +No agent coupling; MinAgent unchanged. Operator finding on 9201: stopping an app via the UI +(Leállítás) raised the global warning banner "Telepített alkalmazás nem fut: … (stopped)" on every +page — including the launcher, where the tile already shows the greyed state — and fired the +`app_start_failed` notification event on the running→down transition. A deliberate user action is not +a fault; it must not alarm the user anywhere. Genuine faults keep alerting exactly as before. + +- **The fix is a one-line filter at the single fix-3 derivation point.** `scanDeployedAppRunStates` + (cmd/controller/main.go) is the only place both the banner dead-list and the notifier Down-set are + computed. Its pure core was extracted to `classifyRunStates([]stacks.Stack)` (testable without a + live Manager), and the down predicate changed from `stacks.IsDownState(st.State)` to + `stacks.IsDownState(st.State) && st.State != stacks.StateStopped`. `StateStopped` is therefore + suppressed from BOTH surfaces: no banner on any page (launcher included) and `Down=false` fed to + the notifier ⇒ no `app_start_failed` event and a clean transition tracker. +- **Why `StateStopped` ⇒ deliberate (two invariants, recorded at the seam and in CONTEXT.md):** + (I1) the UI stop path `Manager.StopStack` runs `docker compose down` → containers are removed, and + a deployed stack with zero containers aggregates to `StateStopped` (refreshStatusLocked). (I2) the + P2 restart-policy census (2026-07-21, 53 templates / 78 services) found every catalog service on + `unless-stopped`, so a crashing app never comes to rest at `stopped` — faults surface as + `restarting` / `unhealthy` / `exited` / `degraded`. **If either invariant changes, revisit this + suppression.** An out-of-band `docker compose stop` leaves containers present → `StateExited` → + still alerts (out-of-band tampering is reportable — acceptable). +- **`IsDownState` deliberately UNCHANGED** — other callers (e.g. `CommittedMemory`, bootrecon) rely + on stopped counting as down. The suppression lives ONLY at the scan; no template, funcmap, + notifier, dashboard-counter, or Hungarian-copy change. The launcher tile still shows greyed + + "Leállítva"; the monitoring page and dashboard RunningCount/StoppedCount are unchanged (factual + display is not an alarm). A pre-existing banner self-clears on the next health cycle (state-based). +- **Tests +4** (notify 3→4, main 4→7): Group A — `classifyRunStates` over [running, stopped, exited, + degraded] yields dead={exited,degraded} and Down flags {false,false,true,true} (red-proof: revert + the filter → both assertions fail, verified). Group B — fault parity: exited+degraded both in the + dead list, both Down=true, raw state string carried through. Group C — stop→start→crash drives + `NotifyAppStartFailures` to exactly ONE event for the crash and zero for the stop (red-proof: mark + the stop Down=true → the zero-for-stop assertion fails, verified). Plus a skip test for + deploying/undeployed. + ### v0.163.1 — Launcher polish: monogram reveal-on-failure + placeholder on every icon surface (2026-07-24) No agent coupling; MinAgent unchanged. Two live findings from the v0.163.0 operator browser pass on 9201. diff --git a/CONTEXT.md b/CONTEXT.md index 231a136..9843174 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -7,7 +7,25 @@ > > Ask Claude Code: "Please update CONTEXT.md with what we did today" -Last updated: 2026-07-24 (v0.163.1 — launcher polish: monogram reveal-on-failure + placeholder everywhere) +Last updated: 2026-07-24 (v0.164.0 — deliberately stopped apps no longer alarm; banner + email suppressed for StateStopped) + +> **2026-07-24 — v0.164.0 (stopped ≠ fault).** Operator finding on 9201: a UI stop (Leállítás) raised +> the global "Telepített alkalmazás nem fut: … (stopped)" banner on every page AND fired the +> `app_start_failed` email. RULING: **a deliberate user action must not alarm anywhere.** One-line +> filter at the single fix-3 derivation point — `scanDeployedAppRunStates`'s pure core extracted to +> `classifyRunStates([]stacks.Stack)`, down predicate now +> `stacks.IsDownState(st.State) && st.State != stacks.StateStopped`. `StateStopped` is dropped from +> BOTH the banner dead-list and the notifier Down-set (⇒ no banner, no event, clean tracker). Rests on +> **two invariants that MUST both hold for this suppression to be correct:** **I1** — the UI stop path +> `Manager.StopStack` runs `docker compose down` → containers removed → a deployed stack with zero +> containers aggregates to `StateStopped` (refreshStatusLocked). **I2** — the P2 restart-policy census +> (2026-07-21, 53 templates / 78 services) found every catalog service on `unless-stopped`, so a crash +> never rests at `stopped` — faults surface as `exited`/`degraded`/`restarting`/`unhealthy`. **If +> either invariant changes, revisit this suppression.** `IsDownState` UNCHANGED (other callers rely on +> stopped=down). Out-of-band `docker compose stop` (containers remain → `StateExited`) still alerts — +> correct, tampering is reportable. The `stopped_by_user` intent flag was considered and PARKED (only +> adds value against out-of-band stops, which should keep alerting). Tests +4 (notify 3→4, main 4→7), +> both red-proofs verified. No template/funcmap/notifier/counter/copy change. > **2026-07-24 — v0.163.1 (launcher polish).** Two v0.163.0 live findings fixed. RULE recorded: > **every app-logo surface ends in a visible placeholder** (`SVG → PNG → /static/app-placeholder.svg`, diff --git a/REUSE.md b/REUSE.md index f808530..419a5aa 100644 --- a/REUSE.md +++ b/REUSE.md @@ -230,6 +230,7 @@ | `Manager.execFn` (func seam) + `restartPolicyLookup` / `inspectRestartPolicyFn` (R-51, v0.156.0) | controller/internal/stacks/manager.go | nil → real `exec.Command` / `docker inspect -f {{.HostConfig.RestartPolicy.Name}}` | `scriptedDocker` in controller/internal/stacks/degraded_test.go drives the WHOLE production path (docker ps → aggregateState → docker inspect) — an aggregateState-only test proves the function, not the caller. Policy answers are cached per container+state and pruned to the live `docker ps` set; a FAILED inspect is deliberately never cached (a hiccup must not pin a container to "unknown") and reads as SUPERVISED, i.e. fail-closed — the opposite of `IsDownState`'s fail-open, because there the state is ambiguous while here a member is known dead | | `bootrecon.StackProvider` (R-52, v0.156.0) | controller/internal/bootrecon/bootrecon.go | `*stacks.Manager` (GetStacks/StartStack/RefreshStatus) | `fakeStacks` counts StartStack per app; the load-bearing assertion is the NEGATIVE — a zero-container stack (a UI Stop = `compose down` = containers removed) must record **0** starts, while a boot orphan (containers present, Exited) records exactly 1. `Reconciler.sleep` is injected so the 30 s gap costs nothing | | `bootReconcileFn` + `runBootReconcile` (package-main seam, v0.156.0) | controller/cmd/controller/main.go | `bootrecon.New(mgr, logger).Run` | controller/cmd/controller/bootrecon_wiring_test.go. **The wiring itself is asserted by an AST walk** over `func main()`, not a `strings.Contains` — the substring version passed its own red-proof because a commented-out call still contains the string. Comments are not callers | +| `classifyRunStates` (pure fix-3 derivation, v0.164.0) | controller/cmd/controller/main.go | `[]stacks.Stack` → `(dead []web.DeadApp, states []notify.AppRunState)`; `scanDeployedAppRunStates` = `classifyRunStates(mgr.GetStacks())` | classify_runstates_test.go. **THE single fix-3 rule: down = `IsDownState(st.State) && st.State != StateStopped`.** A deliberate UI stop (`compose down` → zero containers → StateStopped, I1) must not alarm — banner OR email — while faults (Exited/Degraded) alarm byte-identically; I2 (P2 census: all catalog services `unless-stopped`) is why a crash never rests at stopped. **Do NOT touch `IsDownState`** (other callers rely on stopped=down) and do NOT filter in `buildDeadAppAlerts`/`NotifyAppStartFailures` — one derivation point. If I1 or I2 changes, revisit the suppression | | `report.SetPendingControllerLog` / `SetControllerLogSource` | controller/internal/report/selftail.go | ACK-armed consume-once self-log pull (the logtail.go shape) | selftail_test.go; source = `logBuffer.Lines`, wired once in main.go | | `util.ParseVersion` / `util.Version.Compare` | controller/internal/util/version.go | THE one semver comparator (house rule: never a second) — selfupdate aliases it; agentapi's MinAgent comparison uses it | rejects pre-release/dev/latest (callers fall back, never trust); numeric compare (0.100 > 0.81) | | `agentapi.AgentVersionReporter` + `featureMinAgent` | controller/internal/agentapi/features.go | version-first Supports (v0.82.0 header channel); probe = fallback for header-less agents | a coupled feature adds BOTH a featureProbes row AND a featureMinAgent row; v0.116.0: `SupportsWithSource` also reports HOW the verdict was reached (version/probe-cache/probe) for the gate log line | diff --git a/controller/README.md b/controller/README.md index 77f8b11..2e389f0 100644 --- a/controller/README.md +++ b/controller/README.md @@ -1624,8 +1624,9 @@ Each event carries typed detail structs (e.g., `BackupDetails`, `DiskDetails`, ` **Deployed-app-down alerting (fix-3, v0.120.0, CAMPAIGN-3).** A `deadapp-check` scheduler job (every 30 s, after a 90 s boot grace) scans `stackMgr.GetStacks()`: a DEPLOYED app whose containers are -`stopped`/`exited` (`stacks.IsDownState` — a Docker `created`/`dead` container, the F11 dead-at-boot -case, resolves to `stopped`) gets a **state-based WARN dashboard banner** ("Telepített alkalmazás nem +`exited`/`degraded` (`stacks.IsDownState` minus the `stopped` exclusion added in v0.164.0 — see below; +a Docker `created`/`dead` container, the F11 dead-at-boot case, resolves to `exited`) gets a +**state-based WARN dashboard banner** ("Telepített alkalmazás nem fut: ", grouped above 3 so a reboot storm doesn't wall the dashboard) that self-clears when the app runs again, AND an `app_start_failed` hub event fired **once per running→down transition** (`Notifier.NotifyAppStartFailures` tracks per-app state; down→down cycles are silent — the hub owns @@ -1648,6 +1649,21 @@ colour, counted with the stopped apps, URL flagged unpublished (Traefik withhold routed member is the dead one). Policy reads are one `docker inspect` per down member of a mixed stack, cached per container+state. +**Deliberate stops are silent (v0.164.0).** Stopping an app from the UI (Leállítás → `StopStack` → +`docker compose down` → zero containers → the deployed stack aggregates to `StateStopped`) is the +user's own action, not a fault, and must not raise the banner OR the `app_start_failed` email. The +scan's pure core was extracted to `classifyRunStates([]stacks.Stack)` and its down predicate is now +`stacks.IsDownState(st.State) && st.State != stacks.StateStopped` — the SINGLE fix-3 derivation point, +so `StateStopped` is dropped from both the banner dead-list and the notifier Down-set at once (the +launcher tile still shows greyed „Leállítva"; the monitoring page and dashboard counters are factual +display, not alarms, and are unchanged). This rests on two invariants: **I1** — a UI stop always ends +at `StateStopped` (compose down removes the containers); **I2** — the P2 restart-policy census +(53 templates / 78 services, all `unless-stopped`) means a crashing app never comes to rest at +`stopped`, so faults still surface as `exited`/`degraded`/`restarting`/`unhealthy`. If either +invariant changes, revisit the suppression. `IsDownState` itself is deliberately UNCHANGED (other +callers rely on stopped counting as down). An out-of-band `docker compose stop` leaves the containers +present → `StateExited` → still alerts, which is correct (out-of-band tampering is reportable). + **Boot desired-state reconciliation (R-52, v0.156.0, `internal/bootrecon`).** A `deployed: true` app that missed its boot start used to stay down until a human noticed — the same shutdown that produced F4 left immich and calibre-web `Exited` while ten sibling containers came back, and they were still diff --git a/controller/cmd/controller/classify_runstates_test.go b/controller/cmd/controller/classify_runstates_test.go new file mode 100644 index 0000000..cfc637b --- /dev/null +++ b/controller/cmd/controller/classify_runstates_test.go @@ -0,0 +1,123 @@ +package main + +import ( + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/notify" + "gitea.dooplex.hu/admin/felhom-controller/internal/stacks" + "gitea.dooplex.hu/admin/felhom-controller/internal/web" +) + +// v0.164.0: classifyRunStates is the single fix-3 derivation point. A deliberate user stop +// (StateStopped) must NOT alarm — it is excluded from both the banner dead-list and the notifier +// Down-set — while every genuine fault (StateExited / StateDegraded) keeps alerting byte-identically. +// Invariants behind the suppression are documented at classifyRunStates (I1: compose down ⇒ zero +// containers ⇒ StateStopped; I2: P2 census — all catalog services unless-stopped ⇒ faults never rest +// at stopped). + +func stack(name string, st stacks.ContainerState, deployed, deploying bool) stacks.Stack { + return stacks.Stack{ + Name: name, + Meta: stacks.Metadata{DisplayName: name}, + State: st, + Deployed: deployed, + Deploying: deploying, + } +} + +func downByName(states []notify.AppRunState) map[string]bool { + m := map[string]bool{} + for _, s := range states { + m[s.Name] = s.Down + } + return m +} + +func deadNames(dead []web.DeadApp) map[string]bool { + m := map[string]bool{} + for _, d := range dead { + m[d.Name] = true + } + return m +} + +// Group A (Scenario A) — suppression. Over a [running, stopped, exited, degraded] fixture, the dead +// list is EXACTLY {exited, degraded} and the Down flags are {false, false, true, true}: the stopped +// app is silent, the two faults still alarm. +// +// COMPANION red-proof: revert the filter to bare `stacks.IsDownState(st.State)` (drop the +// `&& st.State != stacks.StateStopped` guard) → stopped reports Down=true and enters the dead list → +// both the dead-set and the Down-flag assertions below fail. (Verified by hand-editing the seam.) +func TestClassifyRunStates_StoppedIsSuppressed(t *testing.T) { + sts := []stacks.Stack{ + stack("radarr", stacks.StateRunning, true, false), + stack("cwa", stacks.StateStopped, true, false), + stack("immich", stacks.StateExited, true, false), + stack("nextcloud", stacks.StateDegraded, true, false), + } + + dead, states := classifyRunStates(sts) + + gotDead := deadNames(dead) + if len(gotDead) != 2 || !gotDead["immich"] || !gotDead["nextcloud"] { + t.Fatalf("dead list must be exactly {immich(exited), nextcloud(degraded)}, got %+v", dead) + } + if gotDead["cwa"] { + t.Errorf("a deliberately stopped app must NOT be in the dead list (no banner)") + } + if gotDead["radarr"] { + t.Errorf("a running app must never be in the dead list") + } + + down := downByName(states) + want := map[string]bool{"radarr": false, "cwa": false, "immich": true, "nextcloud": true} + if len(down) != len(want) { + t.Fatalf("every deployed app must have a run state, got %+v", down) + } + for name, w := range want { + if down[name] != w { + t.Errorf("Down[%s] = %v, want %v (stopped ⇒ false ⇒ no app_start_failed event)", name, down[name], w) + } + } +} + +// Group B (Scenario B) — fault parity. With only exited + degraded present, BOTH surface in the dead +// list AND both report Down=true — byte-identical to v0.163.1 for every non-stopped down state. The +// suppression touches stopped and nothing else. +func TestClassifyRunStates_FaultParity(t *testing.T) { + sts := []stacks.Stack{ + stack("immich", stacks.StateExited, true, false), + stack("nextcloud", stacks.StateDegraded, true, false), + } + + dead, states := classifyRunStates(sts) + + gotDead := deadNames(dead) + if len(gotDead) != 2 || !gotDead["immich"] || !gotDead["nextcloud"] { + t.Fatalf("both faults must appear in the dead list, got %+v", dead) + } + down := downByName(states) + if !down["immich"] || !down["nextcloud"] { + t.Fatalf("both faults must report Down=true, got %+v", down) + } + // State strings must ride through to the banner unchanged (banner shows "(exited)"/"(degraded)"). + byName := map[string]string{} + for _, d := range dead { + byName[d.Name] = d.State + } + if byName["immich"] != string(stacks.StateExited) || byName["nextcloud"] != string(stacks.StateDegraded) { + t.Errorf("dead-app State must carry the raw aggregate state, got %+v", byName) + } +} + +// Deploying and undeployed stacks are skipped entirely (unchanged fix-3 behavior). +func TestClassifyRunStates_SkipsDeployingAndUndeployed(t *testing.T) { + sts := []stacks.Stack{ + stack("mid", stacks.StateDeploying, true, true), // mid-deploy → skipped + stack("gone", stacks.StateExited, false, false), // not deployed → skipped + } + dead, states := classifyRunStates(sts) + if len(dead) != 0 || len(states) != 0 { + t.Fatalf("deploying and undeployed stacks must be skipped, got dead=%+v states=%+v", dead, states) + } +} diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index 5c3a3fb..e53b3f2 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -1161,15 +1161,36 @@ func runBootReconcile(ctx context.Context, mgr bootrecon.StackProvider, logger * // scanDeployedAppRunStates returns the fix-3 view of the deployed apps: the DEAD ones (for the // state-based dashboard banner) and EVERY deployed app's run state (for the notifier's one-event-per- -// transition tracking). Deploying apps are skipped (mid-deploy is not a fault). Pure over GetStacks(). +// transition tracking). Deploying apps are skipped (mid-deploy is not a fault). Pure over GetStacks() +// — the derivation itself lives in classifyRunStates so it is testable without a live Manager. func scanDeployedAppRunStates(mgr *stacks.Manager) ([]web.DeadApp, []notify.AppRunState) { + return classifyRunStates(mgr.GetStacks()) +} + +// classifyRunStates is the pure fix-3 derivation over a plain stack slice. It splits the deployed +// apps into the DEAD list (dashboard banner) and the per-app run states (notifier transition tracker). +// +// v0.164.0: a deliberate user stop is NOT a fault and must not alarm anywhere (banner OR email). The +// down predicate therefore EXCLUDES StateStopped, resting on two invariants: +// - I1: the UI stop path Manager.StopStack runs `docker compose down` → containers are removed, and +// a deployed stack with zero containers aggregates to StateStopped (manager.go refreshStatusLocked). +// So StateStopped means "deployed, deliberately stopped by the user". +// - I2: the P2 restart-policy census (2026-07-21, 53 templates / 78 services) found every catalog +// service on `unless-stopped`, so a crashing app never comes to rest at `stopped` — faults surface +// as StateExited / StateDegraded (and restarting/unhealthy). StateStopped is therefore never a fault. +// +// If either invariant changes, revisit this suppression. (An out-of-band `docker compose stop` leaves +// the containers present → StateExited → still alerts, which is correct: out-of-band tampering IS +// reportable.) IsDownState is intentionally left unchanged — other callers rely on stopped counting as +// down; the suppression is a filter at this single derivation point only. +func classifyRunStates(sts []stacks.Stack) ([]web.DeadApp, []notify.AppRunState) { var dead []web.DeadApp var states []notify.AppRunState - for _, st := range mgr.GetStacks() { + for _, st := range sts { if !st.Deployed || st.Deploying { continue } - down := stacks.IsDownState(st.State) + down := stacks.IsDownState(st.State) && st.State != stacks.StateStopped states = append(states, notify.AppRunState{Name: st.Name, DisplayName: st.Meta.DisplayName, Down: down}) if down { dead = append(dead, web.DeadApp{Name: st.Name, DisplayName: st.Meta.DisplayName, State: string(st.State)}) diff --git a/controller/internal/notify/deadapp_test.go b/controller/internal/notify/deadapp_test.go index 49742d0..44b6348 100644 --- a/controller/internal/notify/deadapp_test.go +++ b/controller/internal/notify/deadapp_test.go @@ -56,6 +56,42 @@ func TestNotifyAppStartFailures_FirstSeenDownFires(t *testing.T) { } } +// Group C (Scenario C, v0.164.0) — stop → start → crash stays correct. The stop is reported as +// Down=false (classifyRunStates suppresses StateStopped — proven in package main's +// TestClassifyRunStates_StoppedIsSuppressed), so it fires NOTHING and leaves the tracker clean; the +// later crash (StateExited → Down=true) is then a clean false→true transition → exactly ONE event. +// +// COMPANION red-proof: flip the stop cycle to Down=true (as bare IsDownState would report before the +// suppression) → the stop becomes a running→down transition and fires here, so the "zero after the +// stop" assertion below fails. (Verified by hand-editing the fixture.) +func TestNotifyAppStartFailures_StopStartCrashSequence(t *testing.T) { + n := New("http://hub", "key", "cust", nil, log.New(io.Discard, "", 0), false) + var count int + n.pushFn = func(eventType, _, _ string, _ interface{}) { + if eventType == "app_start_failed" { + count++ + } + } + + app := "immich" + stopped := []AppRunState{{Name: app, DisplayName: "Immich", Down: false}} // user stop: StateStopped ⇒ Down=false + running := []AppRunState{{Name: app, DisplayName: "Immich", Down: false}} // started again + exited := []AppRunState{{Name: app, DisplayName: "Immich", Down: true}} // then it crashes: StateExited + + // The deliberate stop must be SILENT — the whole point of the suppression. + n.NotifyAppStartFailures(stopped) + if count != 0 { + t.Fatalf("a deliberate stop must fire no event, got %d", count) + } + + // Start, then crash → exactly ONE app_start_failed for the final false→true transition. + n.NotifyAppStartFailures(running) + n.NotifyAppStartFailures(exited) + if count != 1 { + t.Fatalf("stop→start→crash must fire exactly one event for the crash, got %d", count) + } +} + // A deployed app that is up never fires. func TestNotifyAppStartFailures_HealthyNeverFires(t *testing.T) { n := New("http://hub", "key", "cust", nil, log.New(io.Discard, "", 0), false)