diff --git a/CHANGELOG.md b/CHANGELOG.md index c5c38d8..244e434 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,22 @@ +## v0.263.1 — the undo asks the OLD probe even when the new one has marked the app unhealthy (2026-09-23, R-637) + +**MinAgent: 0.131.0** (unchanged). No new strings. + +**Caught by the live proof on 9202, not by a test.** v0.263.0's undo put docmost's data and old +definition back, and then judged the old version "did not start" after the full 90 s — while it was +serving. The periodic health probe judges an app with its CURRENT `.felhom.yml`; the new version had +brought a probe the old one does not answer (the drill's wrong port, and a real case whenever a probe +moves with a version), so it flipped the app to `unhealthy` every 10 s — and the update's health wait +only probes an app whose state reads `running`. The old probe was never asked (`last: state unhealthy`). + +**Fix:** with the undo's override probe, an `unhealthy` app IS probed, and the old check decides; it +is never settled on container state (the settle path still requires `running`). The normal update +path is unchanged. **Why the unit tests missed it:** `TestUndo_UsesTheOldProbe` injects the undo's +whole health wait, so it never reached the state gate. The new test drives the real wait loop with only +the network probe faked (new seam `Manager.probeRunFn`) and reproduces the live message verbatim when +the fix is switched off. The first live attempt ended in an honest HOLD (data put back, old version +not judged healthy) — kept as evidence. + ## v0.263.0 — a failed update puts the app back by itself (2026-09-23, R-637) **MinAgent: 0.131.0** (unchanged). New strings, all born as bundle keys (hu + en): `app_info.update_undone`, diff --git a/controller/internal/stacks/manager.go b/controller/internal/stacks/manager.go index c957f68..61f9360 100644 --- a/controller/internal/stacks/manager.go +++ b/controller/internal/stacks/manager.go @@ -242,6 +242,7 @@ type Manager struct { // which receives the probe it must use (nil ⇒ waitUpdateHealthyMeta). undoCopier volumeCopier updateUndoHealthFn func(ctx context.Context, name string, timeout time.Duration, meta *Metadata) (bool, string) + probeRunFn func(t probeTarget) *HealthProbeResult // the health wait's network probe; nil ⇒ runChecks updateMemoryFn func(newReqMB, newLimitMB, releasedReqMB, releasedLimitMB int) (refusal error, warning string) updateDiskFreeFn func() (freeGiB float64, ok bool) updateNowFn func() time.Time diff --git a/controller/internal/stacks/undo_test.go b/controller/internal/stacks/undo_test.go index b516794..460a8ff 100644 --- a/controller/internal/stacks/undo_test.go +++ b/controller/internal/stacks/undo_test.go @@ -415,3 +415,66 @@ func TestUndo_RemovalDeletesKeptCopies(t *testing.T) { t.Errorf("removed %d, left %d", n, fc.nCopies()) } } + +// TestUndo_OldProbeRunsOnAnAppTheCurrentProbeMarkedUnhealthy drives the REAL wait loop +// (waitUpdateHealthyMeta → RefreshStatus → `docker ps` → the health-probe override → the state gate → +// findProbeContainerMeta) with only the network probe faked. The app's CURRENT .felhom.yml probe has +// failed, so the refresh reads it Unhealthy; the undo's OLD probe answers. +// +// FOUND LIVE, NOT BY A TEST: v0.263.0's first build gated on StateRunning, so the old probe was never +// asked and docmost's undo on 9202 was reported "did not start" after the full 90 s while the old +// version served. TestUndo_UsesTheOldProbe could not see it — it injects updateUndoHealthFn and so +// never reaches this loop (the seam-boundary class: a test proves only what is behind its seam). +// +// COMPANION RED-PROOF (REPORT.md): drop `|| undoProbe` from the state case — the wait times out with +// `last: state unhealthy` and this test fails. +func TestUndo_OldProbeRunsOnAnAppTheCurrentProbeMarkedUnhealthy(t *testing.T) { + m, dir, _, _, _ := newUndoManager(t) + mustWrite(t, filepath.Join(dir, ".felhom.yml"), undoMetaNew) // the catalog's probe (3999) is current + m.mu.Lock() + m.stacks["nextcloud"].Meta = LoadMetadata(dir) + m.stacks["nextcloud"].HealthProbe = &HealthProbeResult{Healthy: false} // the periodic probe failed on 3999 + m.mu.Unlock() + m.execFn = func(name string, args ...string) (string, error) { + if name == "docker" && len(args) > 0 && args[0] == "ps" { + return "nextcloud\tnextcloud:31.0.14-apache\trunning\tUp 30 seconds\tnextcloud\n", nil + } + return "", nil + } + m.inspectRestartPolicyFn = func(string) (string, error) { return "unless-stopped", nil } + m.updateNowFn = time.Now // the shared setup freezes the clock; this loop's deadline needs it moving + var probed []int + m.probeRunFn = func(pt probeTarget) *HealthProbeResult { + probed = append(probed, pt.checks[0].Port) + return &HealthProbeResult{Healthy: pt.checks[0].Port == 3000} + } + if err := m.RefreshStatus(); err != nil { + t.Fatal(err) + } + if st, _ := m.GetStack("nextcloud"); st.State != StateUnhealthy { + t.Fatalf("setup: the refresh must read the app Unhealthy (the current probe failed), got %q", st.State) + } + old, err := savePreUpdateMeta(dir) // what the job kept — but it holds the NEW file here, so write the OLD one + if err != nil { + t.Fatal(err) + } + mustWrite(t, filepath.Join(old, ".felhom.yml"), undoMetaOld) + oldMeta := LoadMetadata(old) + + ok, detail := m.waitUpdateHealthyMeta(context.Background(), "nextcloud", time.Second, &oldMeta) + if !ok { + t.Fatalf("the old version answers its own probe and must be judged healthy; got %q, probed ports %v", detail, probed) + } + if len(probed) == 0 || probed[0] != 3000 { + t.Errorf("the undo must probe the OLD port (3000), probed %v", probed) + } + // And without an override the same Unhealthy app is NOT probed or settled — the normal update path + // is unchanged. + probed = nil + m.mu.Lock() + m.stacks["nextcloud"].HealthProbe = &HealthProbeResult{Healthy: false} // the first call stored its healthy result + m.mu.Unlock() + if ok, _ := m.waitUpdateHealthyMeta(context.Background(), "nextcloud", time.Second, nil); ok || len(probed) != 0 { + t.Errorf("without an override an Unhealthy app must stay unhealthy and unprobed; ok=%v probed=%v", ok, probed) + } +} diff --git a/controller/internal/stacks/update.go b/controller/internal/stacks/update.go index d95a5d4..a113c59 100644 --- a/controller/internal/stacks/update.go +++ b/controller/internal/stacks/update.go @@ -918,11 +918,22 @@ func (m *Manager) waitUpdateHealthyMeta(ctx context.Context, name string, timeou for { _ = m.RefreshStatus() st, ok := m.GetStack(name) + // v0.263.0 — THE UNDO'S PROBE MUST BE ALLOWED TO RUN. The periodic health probe judges the app + // with the CURRENT .felhom.yml and flips a running app to StateUnhealthy when that check fails — + // which is exactly the undo's situation when the new version brought a probe the old one does + // not answer. Gating on StateRunning alone meant the old probe was never asked and the undo was + // reported "did not start" after the full timeout. MEASURED LIVE on 9202 2026-09-23 (docmost, + // `last: state unhealthy` for 90 s while the old version served). So with an override that + // declares checks, an Unhealthy app is PROBED — and the override's own check decides. It never + // settles an Unhealthy app on container state (below: the settle path requires StateRunning). + // Pinned by TestUndo_OldProbeRunsOnAnAppTheCurrentProbeMarkedUnhealthy. + undoProbe := ok && override != nil && st.State == StateUnhealthy && + override.HealthCheck != nil && len(override.HealthCheck.Checks) > 0 switch { case !ok: last = "stack vanished" runningSince = time.Time{} - case st.State == StateRunning: + case st.State == StateRunning || undoProbe: // A declared health check is only usable if it resolves to a container. When it does // not, the app is judged the same way an app with NO declared check is judged — // settling on container state — and the log says so. @@ -942,7 +953,7 @@ func (m *Manager) waitUpdateHealthyMeta(ctx context.Context, name string, timeou c, candidates := findProbeContainerMeta(name, &meta, st.Containers) if c != "" { usable = true - res := m.runChecks(probeTarget{stackName: name, containerName: c, checks: hc.Checks}) + res := m.probeRun(probeTarget{stackName: name, containerName: c, checks: hc.Checks}) m.mu.Lock() if s, ok := m.stacks[name]; ok { s.HealthProbe = res @@ -958,7 +969,9 @@ func (m *Manager) waitUpdateHealthyMeta(ctx context.Context, name string, timeou name, name, candidates) } } - if !usable { + if !usable && st.State != StateRunning { + last = "unhealthy, and the check resolves to no container" + } else if !usable { if runningSince.IsZero() { runningSince = m.now() } @@ -1213,3 +1226,13 @@ func (m *Manager) ResumeInterruptedUpdates(ctx context.Context) int { } return len(names) } + +// probeRun is the network half of the update's health wait — its own seam, so a test can drive the +// REAL wait loop (the state gate, the container resolution, the settle rule) with only the HTTP/TCP +// probe faked. nil ⇒ runChecks. +func (m *Manager) probeRun(t probeTarget) *HealthProbeResult { + if m.probeRunFn != nil { + return m.probeRunFn(t) + } + return m.runChecks(t) +}