v0.205.0 — a run that skipped an app the customer selected is not successful (R-234)
gates / gates (push) Successful in 21s

THE VERDICT. The R-203 block already said "a warning beside a success is read as a
success" and applied it to ONE of the two shapes it describes: an app missing a
declared mandatory FOLDER made the run incomplete, while an app skipped ENTIRELY
still reported ok. Both do now. Which skips count, decided by measurement:
selected+deployed with no recovery unit YES; selected but NOT deployed no (named,
with what to do — a box left amber by an app somebody removed is a status nobody
reads); disconnected/decommissioned drive no (own signal); nothing selected no.
LastSuccess and SnapshotCount still record what WAS captured.

THE FILED MECHANISM WAS NOT THE MEASURED CAUSE, and saying so is the point. §3
stated that toggling an app on leaves it without a bundle so the first run skips
it. Measured on demo-hp: the run's own pre-dump phase calls captureAllRecoveryUnits
for every DEPLOYED stack, through admitApp, before the push — a unit moved aside
was RECREATED and the run reported ok. That state does not survive a run.

What actually produced the 2026-08-06 sequence: the manual run was dropped by the
single-flight while an earlier run was still going. runOffboxBackup returned nil,
the handler had already answered "A tavoli mentes elindult", and the card then
showed the PREVIOUS run's green verdict — read as covering the app just selected.
The decision is now taken synchronously in the handler and a dropped request says
so. The nightly path still returns nil on purpose: nobody asked, and it retries.

§7.3 measured before deciding: CaptureRecoveryUnit writes a few KB of compose +
manifest, only ENUMERATES dumps rather than creating them, is idempotent and does
NOT stop the app — and already runs inside the off-site run. So there is no wait to
remove for a deployed app and NOTHING was built.

28 packages ok, 9/9 gates. Four red-proofs, each asserted to have applied. Fixture
note: the shared provider's ListDeployedStacks returned nil, so Scenario A first
passed for the wrong reason; fixed with an opt-in deployed set that defaults to nil.
This commit is contained in:
2026-08-06 21:58:21 +02:00
parent 53e9bf0224
commit c6b69d888e
11 changed files with 578 additions and 84 deletions
@@ -3,6 +3,7 @@ package web
import (
"context"
"encoding/json"
"errors"
"fmt"
"net/http"
"net/url"
@@ -229,12 +230,28 @@ func (s *Server) offboxRunHandler(w http.ResponseWriter, r *http.Request) {
offboxRedirect(w, r, "A távoli tároló elárvult — előbb indíts új távoli mentést a kártyán látható módon.", true)
return
}
// R-234: the single-flight decision is taken SYNCHRONOUSLY, before the goroutine, so the customer
// is told what actually happened to THEIR request. Deciding it inside the goroutine is what made
// the drop invisible: the handler had already answered „elindult" and the page then showed the
// PREVIOUS run's „✓ Rendben".
// IsRunning() is the CONCURRENCY flag — the very one acquireRunning guards — which is what this
// question is about. (The documented "use RestoreStatus for display" trap is a different question.)
if s.backupMgr.IsRunning() {
s.logger.Printf("[INFO] [web] manual off-box backup NOT started for this request: a run is already in flight")
offboxRedirect(w, r, "Már fut egy távoli mentés — ez a kérés nem indított újat. A most látható eredmény még a korábbi futásé; várd meg, míg ez befejeződik.", true)
return
}
go func() {
ctx, cancel := context.WithTimeout(context.Background(), 3*time.Hour)
defer cancel()
// ...WithProgress: this is the MANUAL trigger, so the page gets live bytes/percent/current app
// (4c). The nightly scheduler keeps calling RunOffboxBackup and stays silent.
if err := s.backupMgr.RunOffboxBackupWithProgress(ctx); err != nil {
if errors.Is(err, backup.ErrOffboxRunInFlight) {
// Lost the race between the check above and acquireRunning — rare, and still not a failure.
s.logger.Printf("[INFO] [web] manual off-box backup dropped by the single-flight (raced)")
return
}
s.logger.Printf("[WARN] [web] manual off-box backup failed: %v", err)
}
}()
@@ -0,0 +1,71 @@
package web
import (
"bytes"
"log"
"net/http/httptest"
"strings"
"testing"
"gitea.dooplex.hu/admin/felhom-controller/internal/settings"
)
// R-234, THE MEASURED CAUSE — a manual run that the single-flight dropped must not be reported as one
// that started.
//
// WHAT WAS MEASURED (Part 4 venue, 2026-08-06). The customer toggled an app on and pressed
// „Távoli mentés most". The handler answered „A távoli mentés elindult — az állapot itt frissül.",
// `runOffboxBackup` hit `acquireRunning`, logged an INFO and returned **nil**, and the card then
// showed the PREVIOUS run's „✓ Rendben · 1 pillanatkép" — which reads as "the app I just selected is
// backed up". It was not: the restore refused for that app minutes later, and only a third run
// carried it.
//
// The verdict fix (R-234 part 1) does not cover this: there was no skipped app in that run, because
// there was no run. A request that did nothing must say so.
//
// Handler-level on purpose: the decision now lives in the handler, before the goroutine, and a
// manager-level assertion cannot observe what the customer was told.
func TestOffboxRunHandler_InFlightRequestIsNotReportedAsStarted(t *testing.T) {
s, sett, m := newOffboxWebServer(t)
if err := sett.SetOffboxTarget(&settings.OffboxTarget{
Enabled: true, Host: "nas.local", Port: 22, User: "felhom", RepoPath: "/srv/repo",
Schedule: "daily", EscrowState: "escrowed",
}); err != nil {
t.Fatal(err)
}
if err := m.WriteOffboxSecrets("PRIVATE-KEY-MATERIAL", "nas.local ssh-ed25519 AAAAhostkey"); err != nil {
t.Fatal(err)
}
if !m.OffboxConfigured() || !m.OffboxRunnable() {
t.Fatal("fixture: the target must be configured and runnable, or the handler exits earlier")
}
// Occupy the single-flight exactly as a run in progress would.
if err := m.AcquireRunningForTest(); err != nil {
t.Fatalf("fixture: %v", err)
}
defer m.ReleaseRunningForTest()
var logbuf bytes.Buffer
s.logger = log.New(&logbuf, "", 0)
w := httptest.NewRecorder()
s.offboxRunHandler(w, httptest.NewRequest("POST", "/backup/offbox/run", nil))
if w.Code != 302 {
t.Fatalf("the handler redirects; got %d", w.Code)
}
loc := w.Header().Get("Location")
if strings.Contains(loc, "elind") {
t.Errorf("a dropped request must NOT be reported as started — that is the defect. Location: %q", loc)
}
if !strings.Contains(loc, "flash_error") {
t.Errorf("it must reach the customer as a problem, not a success flash. Location: %q", loc)
}
// It must also say the visible result belongs to the EARLIER run — that is what was misread.
if !strings.Contains(loc, "kor%C3%A1bbi") {
t.Errorf("the message must say the shown result is the earlier run's. Location: %q", loc)
}
if !strings.Contains(logbuf.String(), "NOT started") {
t.Errorf("the drop must be findable in the log too, got %q", logbuf.String())
}
}