diff --git a/CHANGELOG.md b/CHANGELOG.md index feabc14..09207b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,54 @@ +## v0.204.0 — what you can restore is decided by the store, not by what happens to be installed (2026-08-06, R-237 / R-238) — MinAgent 0.127.0 + +**A household that had just lost its box was shown nothing to restore.** Measured live on the R-201 +re-walk (`felhom.eu/documentation/tests/part4-rewalk-2026-08-06/journal.md`): after a rebuild, with +the key recovered, the tier configured and the escrow re-sealed, `/backups/restore` said „Nincs +telepített alkalmazás" and the wizard refused every app with „Ez az alkalmazás nincs távoli mentésre +kijelölve" — while the repository held their snapshots the whole time. + +**The list was keyed on the wrong thing.** It was `buildOffboxApps()` filtered on `.Enabled`: apps +**currently deployed** AND **currently toggled on for FUTURE off-site backups**. A rebuilt box has +neither. That is a circular dead end at the worst possible moment — to restore an app you must select +it, to select it you must have installed it, and to know what to install you must see the backup you +cannot see. **The toggle is a statement about future backups; requiring it to look at a past one +conflates two different questions, and that conflation was the defect.** + +**The store is now the source of the list** (`internal/web/offsite_restore_list.go`), built on the +existing R-193 inventory (`OffsiteInventoryList`) which already reads the repository and groups by +app tag. Installed-ness became a property OF a row, never a filter on it: it changes what restoring +implies, not whether the row exists. Every case is answered rather than hidden — + +| case | what the customer sees | +|---|---| +| snapshot present, app NOT installed | listed and restorable, plus „Nincs telepítve — a visszaállítás előbb újratelepíti." | +| app installed, no snapshot | listed, „Nincs mentése a távoli tárolóban — nincs mit visszaállítani." | +| store unreadable | „Nem tudjuk elolvasni a távoli tárolót, ezért **nem tudjuk, mi van benne**. Ez nem azt jelenti, hogy üres…" — **and the action is still offered**, because "we could not look" is not "there is nothing" | +| no target yet (the pristine rebuilt shape) | „A távoli tároló kapcsolódási adatai még nem érkeztek meg ehhez a géphez… Ez magától rendeződik." | +| store genuinely empty | „A távoli tároló üres — nincs mit visszaállítani." | + +That unknown-is-not-empty rule is **R-225's, one screen over**, and it now points both ways: a read +failure must not be rendered as an empty list, and it must not silently withhold the action either. + +**Two marker tags are excluded from the app list**: `felhom-offbox` rides on every snapshot, and +`_shares` has its own restore entry with no per-app wizard — listing either would have offered a +restore of something that does not exist. (The R-193 unlock listing still shows them; that is +recorded, not fixed here.) + +### R-238 — the size gate no longer refuses in silence + +**Classified as a harness artifact, and the residue fixed anyway.** `POST /backup/offbox/restore` +with `mode=full` and no `confirm=1` is step 1 of a deliberate two-step: it computes size + headroom, +**starts no job**, and redirects carrying `&full_prep=` so `deriveWizardStep` reveals the +commit. A driver that does not carry that parameter forward lands back on the intent step — which is +`deriveWizardStep` working exactly as its precedence comments describe, and is why the endpoint-level +run read as "the button does nothing". **The operator's browser run completed the same restore.** + +**What was genuinely wrong: neither branch of that step wrote anything to the log.** `restore-status` +is empty by design (no job), and `offboxRedirectTo` only flashes to the page — so a customer refused +a disaster restore, **including a refusal by the headroom gate**, left no trace on the box at all. +Both branches now log, and so does the concurrent-op refusal. Nothing about the wizard's precedence +rules was re-keyed: a stale parameter must still never resurrect a commit button mid-restore. + ## v0.203.0 — the box collects what the hub staged for it (2026-08-06, R-218 consume half / R-220 message) — MinAgent 0.127.0 **R-218's declaration half shipped in v0.201.0 and works. Its consume half never existed.** diff --git a/REPORT.md b/REPORT.md index 1be6eb2..6a938af 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,101 +1,91 @@ -# REPORT — felhom-controller v0.203.0 (+ felhom-agent v0.127.0) +# REPORT — v0.204.0: the restore list is keyed on the store (R-237), and the size gate stops refusing in silence (R-238) -**The two dead ends that kept the recovery journey failing are closed in code.** R-218's consume half -and R-220 (both halves: the host-side detection and the customer-facing refusal). +2026-08-06. Controller **v0.203.0 → v0.204.0**. MinAgent unchanged (**0.127.0**) — nothing here needs +a new agent capability. `felhom-agent` untouched. -## 1. R-218 — which retry shape, and why (§7.1) +## R-238 — classified: a HARNESS ARTIFACT, with a real residue that is fixed -**Poll**, on a 5-minute scheduler tick, driven from the box's own published declaration -(`OffboxReportStatus().State`) — **the very statement the hub acts on**, so the two cannot disagree -about whether a retry is wanted. +The two runs diverge at one parameter, and the state at that point is quoted rather than inferred: -**Decided on Scenario B, measured against the actual promises:** +- `POST /backup/offbox/restore` with `mode=full` and **no** `confirm=1` is **step 1 of a deliberate + two-step** (`offbox_handlers.go:314`). It computes size + headroom via `OffboxRestorePrepareFull`, + **starts no job**, and redirects to + `restoreWizardPath(app) + "&full_prep=&full_size="`. +- `deriveWizardStep` (`restore_wizard.go`) reveals the commit **only** when + `in.FullPrepApp == in.App`, sourced from `?full_prep=`. +- The endpoint-level driver posted step 1 and then re-fetched the wizard **without** that parameter. + The pure function therefore returned the **intent** step — correctly. `restore-status.last == null` + is likewise **correct**: step 1 starts no job by design. +- The operator's browser run followed the redirect, saw the confirm, pressed „Igen" twice, and the + restore completed. -| surface | what it promises | +**So the button is not dead, and the wizard was not re-keyed.** The precedence rules exist so a stale +`?full_prep=` can never resurrect a commit button mid-restore, and they were left alone. + +**The residue, which is real whoever triggers it:** neither branch of step 1 wrote anything to the +log. `offboxRedirectTo` only flashes to the page. A customer refused a disaster restore — **including +a refusal by the headroom gate** — left **no trace on the box at all**. Fixed: the refusal logs +`[WARN] … full-restore preparation REFUSED for (no job started): `, the success logs +`[INFO] … full-restore prepared for (size N) — awaiting the customer's confirm; no restore has +started`, and the concurrent-op refusal logs too. + +## R-237 — the restore list, rebuilt on the store + +`buildOffsiteRestoreRows` (new, pure) merges `OffsiteInventoryList` (the repository's own snapshot +tags — the existing R-193 reader) with the installed set. `resolveOffsiteRestoreApp` replaces +`resolveWizardApp`'s toggle requirement. + +Every §7 case, and the Hungarian as rendered: + +| case | rendered | |---|---| -| the no-target unlock message | „…**amint megvannak**, a mentéseid listája megjelenik…" — **no deadline** | -| the backups card | „…**Ha egy napon belül nem áll be**, jelezd az üzemeltetőnek." — **within a day** | +| snapshot present, app not installed | restore offered + „Nincs telepítve — a visszaállítás előbb újratelepíti." | +| installed, no snapshot | „Nincs mentése a távoli tárolóban — nincs mit visszaállítani." | +| store unreadable | „Nem tudjuk elolvasni a távoli tárolót, ezért **nem tudjuk, mi van benne**. Ez nem azt jelenti, hogy üres — próbáld újra később, vagy jelezd az üzemeltetőnek." + „Nem tudjuk, van-e mentése — a tárolót nem sikerült elolvasni." **and the action stays offered** | +| no target yet | „A távoli tároló kapcsolódási adatai még nem érkeztek meg ehhez a géphez, ezért még nem tudjuk megmutatni, mi van benne. Ez magától rendeződik." | +| store empty | „A távoli tároló üres — nincs mit visszaállítani." | +| app under a different name | **not guessed** — it lists under the tag the store holds, and if nothing is installed under that name the row says so. No fuzzy matching. | -A 5-minute tick is inside both by a wide margin, and it needs **no hub change** (which §0 expected). -**No text needed changing.** If either promise ever tightens to minutes, the ACK-driven shape is the -one to reach for. +Wizard refusals also changed: „Ehhez az alkalmazáshoz nincs mentés a távoli tárolóban." and, when the +store could not be read, „Nem tudjuk elolvasni a távoli tárolót, ezért nem tudjuk, van-e benne mentés +ehhez az alkalmazáshoz." Both log an INFO naming the app and the store state. -**It stops by construction** (Scenario C) and **the settle gate is kept** (Scenario D) — the retry goes -through `ReconcileWhenSettled`, unchanged. +`felhom-offbox` and `_shares` are excluded from the app list. -## 2. The marker question (§7.2) — answered, and left alone +## Tests -`applied_marker` lives at `/offbox/applied_marker` — **inside the guest's data dir, which a -rebuild destroys.** It therefore cannot suppress a legitimate post-rebuild re-run, and it is not part -of this defect. Untouched. +`go build` · `go vet` · `go test ./...` → **28 packages ok**. `controller_gates.py --fast` → **9/9 OK**. -## 3. R-220's shape (§7.3) — corroboration, not a wider prefix +New: `offsite_restore_list_test.go` (7 tests — the truth table, the rebuilt box, unreadable-is-unknown, +no-target, empty, and two rendered-page tests) and `offbox_restore_silence_test.go` (handler-level, +because the silence was in the handler). -**Candidate (b)**, the claimed check distinguishing our own mount from a foreign one — because the host -and its knowledge survive the rebuild while the guest's registry does not. Candidate (a) (a cleanup -that clears the raw mounts) would have made correctness depend on a step running, and a step that does -not run leaves exactly today's defect. +## Red-proofs — each mutation asserted to have applied before the result was trusted -**The discriminator is corroborated:** a non-managed mountpoint is forgiven **only when the same device -is ALSO mounted under `/mnt/felhom-drives`** — a pairing only our enrolment produces. -**`/proc/mounts`, not `lsblk MOUNTPOINTS`**, because the lsblk invocation is pinned verbatim in the -sudoers file and changing it would have coupled this fix to a config rollout. +| # | mutation | result | +|---|---|---| +| RP-1 | store rows dropped from the builder — the list keyed back on installed apps (the original defect) | `TruthTable` + `RebuiltBox_SeesItsSnapshots` **FAIL** | +| RP-2 | `state` forced to `known` **and** `StoreUnknown` forced false (both guards) | `UnreadableStoreIsUnknownNotEmpty` + `NoTargetIsItsOwnState` **FAIL** | +| RP-3 | both size-gate log lines removed | `FullPrepareRefusal_IsNotSilent` **FAIL** — „wrote NOTHING to the log" | -**The fence holds and has its own test** (Scenario F): `/srv/data`, `/media/photos`, -`/mnt/someone-elses-disk`, `/var/lib/other` are all still refused. **Fail-safe:** an unreadable mount -table corroborates nothing. +All three restored; suite green again afterwards. -## 4. Every changed Hungarian string +## Files -**The deploy refusal (R-220, Scenario G).** Was: +- new `internal/web/offsite_restore_list.go`, `internal/web/offsite_restore_list_test.go`, + `internal/web/offbox_restore_silence_test.go` +- `internal/web/restore_wizard.go` (gate on the store, not the toggle) +- `internal/web/offbox_handlers.go` (three log lines) +- `internal/web/handlers.go` (wire the rows) +- `internal/web/templates/backups_restore.html` (the list + the state wording) +- `internal/web/backups_split_test.go` (fixture: the new data contract) +- `internal/backup/offbox_inventory.go` (`ErrNoOffsiteTargetSentinel`, so the no-target case is + constructible from another package's table test) +- `CHANGELOG.md`, `controller/README.md` -> „A megadott tárhely nem azonosítható regisztrált meghajtóként, ezért alkalmazás adatkönyvtáraként nem -> használható. **Válasszon a listából csatlakoztatott meghajtót.**" +## Not done here, deliberately -Now: - -> „A megadott tárhely nem azonosítható regisztrált meghajtóként, ezért alkalmazás adatkönyvtáraként nem -> használható. **Ha a gépet nemrég telepítettük újra, a meghajtóid megvannak, de még nincsenek újra -> csatlakoztatva ehhez a géphez — a Tárhely → Meghajtók oldalon csatlakoztathatod őket, és utána -> indítsd újra a telepítést. Ha ott sem látszanak, keresd a Felhom ügyfélszolgálatát.**" - -**No other customer-facing string changed.** The NAS refusal is a different situation and is untouched. - -## 5. Tests and red-proofs — every one demonstrated failing, then restored - -| Scenario | Test | Red-proof mutation | Result | -|---|---|---|---| -| **A** | a credential staged after start-up is collected | make `RetryIfDeclared` return `(false, nil)` — the pre-v0.203.0 world | **FAILS**: "the box declared a need and no reconcile was attempted" | -| **C** | a healthy box does nothing, and logs nothing | drop the `!declared()` stop condition | **FAILS**: a healthy box reconciles | -| **D** | the settle gate holds on the retry path | call `Reconcile` instead of `ReconcileWhenSettled` | **FAILS**: "SETTLE GATE BYPASSED" | -| **E** | the customer's own drive is offered after a rebuild | remove the `felhomOwnedMounts` exemption | **FAILS**: "device is mounted at /mnt/adatok (sdb)" | -| **F** | a genuinely foreign mount is still refused | over-widen to any `/mnt/*` path | **FAILS**: "/mnt/someone-elses-disk was offered for formatting" | -| **G** | an empty list explains itself | restore the old sentence | **FAILS**: "the refusal tells the customer to choose from a list that may be empty" | - -Plus: a nil bridge is a silent no-op; the corroboration requires both mounts of the **same** device, -and another device's managed mount does not vouch for a foreign one. - -**Green:** controller **28 packages ok**, agent **29 packages ok**, both `go vet` clean, all gates OK -in both repos. - -## 6. Live checks (§7.4) - -**Deployed to demo-hp** from the **published** bytes (agent sha verified against the artifact before -installing): agent **0.127.0**, controller **0.203.0**, healthy. - -**Scenario C — PROVEN LIVE, with a positive control.** The retry job is registered -(`Registered periodic job: offsite-credential-retry (every 5m0s)`), **ran once** -(`Running job: offsite-credential-retry` = 1) and produced **zero** log lines on a box whose tier is -configured. The positive control matters: without it, silence could have meant the job never fired. -*(A first reading counted 2 lines and looked like noise; they were the START-UP reconcile's settle-gate -lines, not the retry job — the instrument was corrected before the conclusion.)* - -**Scenarios A and E are NOT live-proven yet, and deliberately so.** Both need a *rebuilt* box: A needs -one declaring `needs_credential` (no target), E needs one whose registry lost its drives while the raw -mounts survived. Contriving either on a demo box means dismantling its real off-site state; the state -arises **naturally in Part 4**, which is where they belong and where the last re-walk measured them. - -## 7. Not changed - -No hub change. No sudoers, no allowlisted command, no PVE surface. The settle gate, the marker, every -other claim signal, and the NAS refusal are untouched. +- **R-236 is diagnosed, not fixed** — the fix's shape depends on the diagnosis, and this arc has twice + shipped a fix aimed at the wrong half of a defect. +- The R-193 unlock listing still shows the `felhom-offbox` marker as if it were an app. Noticed while + reusing its reader; **out of scope and not touched.** diff --git a/controller/README.md b/controller/README.md index fe804e0..3052304 100644 --- a/controller/README.md +++ b/controller/README.md @@ -154,6 +154,12 @@ backups, monitoring and notifications. All Proxmox/disk operations are delegated action block right; used by the dashboard installed-apps list, the Távoli mentés toggle list and the Visszaállítás restore-to-verify/.fab lists; the backups-apps expander header is ALIGNED to the same grammar (own markup — it carries the toggle). Protected infra stacks + **The off-site restore list is keyed on the STORE (v0.204.0, R-237):** `offsite_restore_list.go` + builds it from `OffsiteInventoryList` (the repository's own snapshot tags), NOT from deployed + + offsite-toggled apps. A rebuilt box has neither and used to be shown nothing to restore while its + snapshots sat in the repository. Installed-ness is a property OF a row (it changes what restoring + implies), never a filter on it; an unreadable store renders as UNKNOWN and keeps the action; the + `felhom-offbox` and `_shares` marker tags are never offered as apps. (traefik/cloudflared/filebrowser) get curated Hungarian display identity from the `inframeta.go` map (name + description + generic `/static/infra-logo.svg` fallback icon); filebrowser is the only infra stack with a customer link (`files.`). diff --git a/controller/internal/backup/offbox_inventory.go b/controller/internal/backup/offbox_inventory.go index 1a1f2e6..1d7718f 100644 --- a/controller/internal/backup/offbox_inventory.go +++ b/controller/internal/backup/offbox_inventory.go @@ -29,6 +29,12 @@ var errNoOffsiteTarget = errors.New("no off-site target is configured on this bo // thing rather than showing a generic failure. func ErrNoOffsiteTarget(err error) bool { return errors.Is(err, errNoOffsiteTarget) } +// ErrNoOffsiteTargetSentinel exposes the sentinel itself so other packages — and their tests — can +// construct the not-yet-configured case. Added for R-237, whose restore list must distinguish +// "no target yet" (resolves by itself) from "could not read" (does not), and must be able to pin +// both in a table test. +func ErrNoOffsiteTargetSentinel() error { return errNoOffsiteTarget } + // OffsiteInventoryApp is one app's presence in the opened repository. Non-secret throughout. type OffsiteInventoryApp struct { App string // the restic tag == the stack name diff --git a/controller/internal/web/backups_split_test.go b/controller/internal/web/backups_split_test.go index 22330de..29bef99 100644 --- a/controller/internal/web/backups_split_test.go +++ b/controller/internal/web/backups_split_test.go @@ -28,6 +28,12 @@ func splitTestData() map[string]interface{} { "OffboxConfigured": true, "OffboxApps": []OffboxAppRow{{Name: "calibre-web", DisplayName: "Calibre-Web", Enabled: true}}, "OffboxToggledCount": 1, + // R-237: the restore list is driven by the STORE. The happy fixture is "the app is in the + // repository AND installed here" — the pre-rebuild shape. + "OffsiteRestoreRows": []OffsiteRestoreRow{ + {App: "calibre-web", DisplayName: "Calibre-Web", InStore: true, Installed: true, Enabled: true}, + }, + "OffsiteStoreState": string(offsiteStoreKnown), "OffboxQuotaPct": 0, "GuestBackup": map[string]interface{}{"Available": false, "Note": "n/a"}, } diff --git a/controller/internal/web/handlers.go b/controller/internal/web/handlers.go index 99d00d3..2b3c4ed 100644 --- a/controller/internal/web/handlers.go +++ b/controller/internal/web/handlers.go @@ -1007,6 +1007,13 @@ func (s *Server) backupsRestoreHandler(w http.ResponseWriter, r *http.Request) { if s.backupMgr != nil { data["OffsiteRestoreCopies"] = s.backupMgr.ListOffsiteRestoreCopies() } + // R-237: the restore list is driven by the STORE, not by what is deployed and toggled. A rebuilt + // box has neither, and used to be shown nothing at all while its snapshots sat in the repository. + if s.backupMgr != nil { + rows, state := s.offsiteRestoreRows(r.Context()) + data["OffsiteRestoreRows"] = rows + data["OffsiteStoreState"] = string(state) + } s.executeTemplate(w, r, "backups_restore", data) } diff --git a/controller/internal/web/offbox_handlers.go b/controller/internal/web/offbox_handlers.go index 3717aa3..7473f1d 100644 --- a/controller/internal/web/offbox_handlers.go +++ b/controller/internal/web/offbox_handlers.go @@ -316,9 +316,18 @@ func (s *Server) offboxRestoreHandler(w http.ResponseWriter, r *http.Request) { defer cancel() sizeHuman, err := s.backupMgr.OffboxRestorePrepareFull(pctx, app) if err != nil { + // R-238: this leg starts no job, so it appears NOWHERE in the restore-op status. Until + // v0.204.0 it also logged nothing, which made a refused disaster restore — including a + // refusal by the headroom gate — completely invisible on the box: no error, no line, and + // a redirect that lands the customer back where they started. Diagnosing a silence is + // what this project has spent a fortnight removing. + s.logger.Printf("[WARN] [web] off-box full-restore preparation REFUSED for %s (no job started): %v", app, err) offboxRedirectTo(w, r, restoreWizardPath(app), err.Error(), true) return } + // The success half is logged too: it is the step that decides the customer may proceed, and + // "the size gate passed at N" is the line that explains the confirm they were then shown. + s.logger.Printf("[INFO] [web] off-box full-restore prepared for %s (size %s) — awaiting the customer's confirm; no restore has started", app, sizeHuman) // R-48: the size-gate reveal now lands on the app's wizard (prepare-confirm step) rather than // on the list page. Same params, same meaning — only the surface that renders them changed. http.Redirect(w, r, restoreWizardPath(app)+"&full_prep="+url.QueryEscape(app)+"&full_size="+url.QueryEscape(sizeHuman), http.StatusFound) @@ -327,6 +336,8 @@ func (s *Server) offboxRestoreHandler(w http.ResponseWriter, r *http.Request) { // Fast-path refuse a concurrent op, then run async on a BACKGROUND context (a proxy read-timeout on // r.Context() would CANCEL the SFTP restore mid-flight — the F4 lesson). if s.backupMgr.IsRunning() { + // Same silence class as the size gate: a refusal that starts nothing must still be findable. + s.logger.Printf("[WARN] [web] off-box restore refused for %s (mode=%s): another backup/restore op is already running", app, mode) offboxRedirectTo(w, r, restoreWizardPath(app), "Egy mentési/visszaállítási művelet már fut.", true) return } diff --git a/controller/internal/web/offbox_restore_silence_test.go b/controller/internal/web/offbox_restore_silence_test.go new file mode 100644 index 0000000..5f2e0ca --- /dev/null +++ b/controller/internal/web/offbox_restore_silence_test.go @@ -0,0 +1,72 @@ +package web + +import ( + "bytes" + "context" + "errors" + "log" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// R-238 — the full-restore size gate must never refuse in silence. +// +// WHAT WAS MEASURED (2026-08-06, part4 venue). `POST /backup/offbox/restore` with `mode=full` and no +// `confirm=1` is step 1 of a deliberate two-step: it computes size + headroom, starts NO job, and +// redirects carrying `&full_prep=` so the wizard reveals the commit. Driving it without +// carrying that parameter forward lands back on the intent step — which is correct behaviour, and is +// why the endpoint-level run looked like "the button does nothing". +// +// The REAL defect underneath, and the one this pins: neither branch of that step wrote anything to +// the controller's log. `restore-status` is empty by design (no job), the redirect is invisible, and +// `offboxRedirectTo` only flashes to the page — so a customer refused a disaster restore, INCLUDING +// a refusal by the headroom gate, left no trace on the box at all. "No error, no log line" is a +// diagnosis problem whoever triggers it. +// +// Handler-level on purpose: the silence was in the handler, and a helper-level assertion cannot +// observe it. That mistake has been made three times in this arc. +func TestOffboxRestore_FullPrepareRefusal_IsNotSilent(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() { + t.Fatal("fixture: the target must be configured, or the handler exits before the size gate") + } + // Deterministic refusal: every restic call fails, so the size gate errors for a real reason + // instead of being skipped, and no network is involved. + m.SetOffboxRunner(func(ctx context.Context, env []string, args ...string) ([]byte, error) { + return nil, errors.New("repository unreachable") + }) + + var logbuf bytes.Buffer + s.logger = log.New(&logbuf, "", 0) + + form := url.Values{"app": {"calibre-web"}, "mode": {"full"}} + req := httptest.NewRequest("POST", "/backup/offbox/restore", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + w := httptest.NewRecorder() + s.offboxRestoreHandler(w, req) + + if w.Code != 302 { + t.Fatalf("the size gate redirects; got %d", w.Code) + } + got := logbuf.String() + // The CONSEQUENCE, not the mechanism: whatever the outcome, this step must be findable in the log. + if !strings.Contains(got, "full-restore preparation REFUSED") && !strings.Contains(got, "full-restore prepared") { + t.Errorf("the full-restore size gate wrote NOTHING to the log \u2014 that is R-238's residue.\nlog was: %q", got) + } + if !strings.Contains(got, "calibre-web") { + t.Errorf("the line must name the app it refused, got %q", got) + } +} diff --git a/controller/internal/web/offsite_restore_list.go b/controller/internal/web/offsite_restore_list.go new file mode 100644 index 0000000..09062ea --- /dev/null +++ b/controller/internal/web/offsite_restore_list.go @@ -0,0 +1,185 @@ +package web + +import ( + "context" + "errors" + "sort" + "time" + + "gitea.dooplex.hu/admin/felhom-controller/internal/backup" +) + +// R-237 — what a customer may restore is decided by WHAT IS IN THE STORE, not by what happens to be +// installed. +// +// WHY THIS EXISTS. Until v0.204.0 the off-site restore list was `buildOffboxApps()` filtered on +// `.Enabled`: apps that are **currently deployed** AND **currently toggled on for future off-site +// backups**. Both halves are wrong for the one situation the list exists to serve. Measured on the +// R-201 re-walk (2026-08-06, `documentation/tests/part4-rewalk-2026-08-06/journal.md`): after a +// rebuild the box had no deployed stacks, so the page said „Nincs telepített alkalmazás" and the +// wizard refused every app with „Ez az alkalmazás nincs távoli mentésre kijelölve" — while the +// repository held their snapshots the whole time. +// +// That is a circular dead end at the worst possible moment: to restore an app you must select it, to +// select it you must have installed it, and to know what to install you must see the backup you +// cannot see. A household that has just lost its box does not know what it used to run. +// +// The toggle is a statement about FUTURE backups. Requiring it to look at a PAST one conflates two +// different questions, and that conflation is the defect. +// +// THE RULE, stated where it is implemented: the store is the source of the list. Installed-ness is a +// property OF a row, never a filter on it — it changes what restoring implies, not whether the row +// exists. + +// offboxMarkerTag is on EVERY off-site snapshot (`restic backup --tag felhom-offbox --tag `), +// so it appears in the tag set beside the real app names. It is a marker, not an app, and listing it +// would offer the customer a restore of something that does not exist. +const offboxMarkerTag = "felhom-offbox" + +// offsiteStoreState is what we know about the repository itself, kept separate from the rows so that +// "we could not read it" can never be rendered as "there is nothing in it". +// +// This is R-225's rule one screen over: an unknown is not a zero. The remote-backup card already +// learned to say „még nem tudjuk" rather than 0; a restore list that renders a read failure as an +// empty list makes the same false claim about something more consequential. +type offsiteStoreState string + +const ( + // offsiteStoreKnown — the repository was read. The rows are the truth. + offsiteStoreKnown offsiteStoreState = "known" + // offsiteStoreUnreadable — the repository could not be read. Rows may still list installed apps, + // but the page MUST say the store's contents are unknown. + offsiteStoreUnreadable offsiteStoreState = "unreadable" + // offsiteStoreNoTarget — no off-site target is configured yet. Distinguished from unreadable + // because it resolves by itself once the tier is (re-)applied, which is the pristine rebuilt + // shape (R-236). + offsiteStoreNoTarget offsiteStoreState = "no-target" +) + +// OffsiteRestoreRow is one restorable thing, from the union of "in the store" and "installed here". +type OffsiteRestoreRow struct { + App string + DisplayName string + Slug string + // InStore — a snapshot in the repository carries this app's tag. This is what makes the row + // restorable at all. + InStore bool + // Installed — the app is deployed on this box right now. NOT a filter: it decides what restoring + // implies (restore in place vs. reinstall first), which the page states rather than hiding. + Installed bool + // Enabled — the app is toggled on for FUTURE off-site backups. Carried for display only; it must + // never gate a restore. + Enabled bool + // StoreUnknown — the repository could not be read, so InStore is not a claim about anything. The + // row keeps its action: refusing to offer a restore because we could not look would be the SAME + // false claim as rendering the read failure as an empty list, pointed the other way. The customer + // is told we could not read it; the attempt then fails honestly rather than being pre-empted. + StoreUnknown bool + LatestAt time.Time + SizeBytes int64 +} + +// Restorable reports whether a restore may be OFFERED for this row: the store holds it, or we could +// not read the store and must not pretend that means "no". +func (r OffsiteRestoreRow) Restorable() bool { return r.InStore || r.StoreUnknown } + +// buildOffsiteRestoreRows merges the repository's contents with the installed set. +// +// Pure on purpose: every §7 edge case (a snapshot with no app, an app with no snapshot, an unreadable +// store, the marker tags) is a row in one table test rather than a live-repository fixture. +// +// invErr is classified, not swallowed: a not-yet-configured target and a failed read are different +// answers to the customer and the page says which. +func buildOffsiteRestoreRows(inv backup.OffsiteInventory, invErr error, installed []OffboxAppRow) ([]OffsiteRestoreRow, offsiteStoreState) { + state := offsiteStoreKnown + if invErr != nil { + state = offsiteStoreUnreadable + if backup.ErrNoOffsiteTarget(invErr) { + state = offsiteStoreNoTarget + } + } + + byApp := map[string]*OffsiteRestoreRow{} + + // The store first — it is the source of the list. + if invErr == nil { + for _, a := range inv.Apps { + if a.App == offboxMarkerTag || a.App == backup.SharesPseudoStack { + // The marker is not an app; shares have their own entry with their own restore path + // and no per-app wizard, so a synthetic row here would be a button that cannot work. + continue + } + byApp[a.App] = &OffsiteRestoreRow{ + App: a.App, DisplayName: a.App, InStore: true, + LatestAt: a.LatestAt, SizeBytes: a.SizeBytes, + } + } + } + + // Then the installed set — it supplies display names and slugs, and contributes rows of its own so + // that "installed, but nothing to restore" is SHOWN rather than silently absent. + for _, ia := range installed { + row, ok := byApp[ia.Name] + if !ok { + row = &OffsiteRestoreRow{App: ia.Name} + byApp[ia.Name] = row + } + row.Installed = true + row.Enabled = ia.Enabled + if ia.DisplayName != "" { + row.DisplayName = ia.DisplayName + } + row.Slug = ia.Slug + } + + out := make([]OffsiteRestoreRow, 0, len(byApp)) + for _, r := range byApp { + r.StoreUnknown = state != offsiteStoreKnown + out = append(out, *r) + } + // Restorable rows first (that is what the page is for), then alphabetically — a stable order, so + // the list does not reshuffle between reloads. + sort.Slice(out, func(i, j int) bool { + if out[i].InStore != out[j].InStore { + return out[i].InStore + } + return out[i].App < out[j].App + }) + return out, state +} + +// offsiteRestoreRows reads the repository and merges it with the installed set. One `restic +// snapshots --json` behind the manager's own probe timeout; a read failure is CLASSIFIED and +// returned, never swallowed into an empty list. +func (s *Server) offsiteRestoreRows(ctx context.Context) ([]OffsiteRestoreRow, offsiteStoreState) { + installed := s.buildOffboxApps() + if s.backupMgr == nil { + return buildOffsiteRestoreRows(backup.OffsiteInventory{}, errNoBackupManager, installed) + } + inv, err := s.backupMgr.OffsiteInventoryList(ctx) + if err != nil && !backup.ErrNoOffsiteTarget(err) { + // Loud on purpose: an unreadable store is the case that used to render as "empty", and the + // customer-facing wording depends on this being distinguishable in the log too. + s.logger.Printf("[WARN] [web] offsite restore list: repository unreadable — listing it as UNKNOWN, not empty: %v", err) + } + return buildOffsiteRestoreRows(inv, err, installed) +} + +// errNoBackupManager stands in for "this box cannot answer" so the page says unknown rather than +// empty when the manager is absent (tests, and the brief window before wiring). +var errNoBackupManager = errors.New("backup manager unavailable") + +// resolveOffsiteRestoreApp finds the wizard's app among the rows. +// +// It deliberately does NOT require the future-backup toggle — that gate is what R-237 is about. It +// requires the row to be RESTORABLE, because a wizard for an app with no snapshot would be a page of +// controls with nothing behind them. +func resolveOffsiteRestoreApp(rows []OffsiteRestoreRow, name string) *OffsiteRestoreRow { + for i := range rows { + if rows[i].App == name && rows[i].Restorable() { + cp := rows[i] + return &cp + } + } + return nil +} diff --git a/controller/internal/web/offsite_restore_list_test.go b/controller/internal/web/offsite_restore_list_test.go new file mode 100644 index 0000000..24b5404 --- /dev/null +++ b/controller/internal/web/offsite_restore_list_test.go @@ -0,0 +1,173 @@ +package web + +import ( + "errors" + "strings" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-controller/internal/backup" +) + +// R-237 — the truth table for "what may this customer restore". Every §7 edge case is a row here +// rather than a live-repository fixture, because the case that matters most (a REBUILT box: nothing +// installed, nothing toggled, snapshots present) is precisely the one that is hard to stand up live +// and was therefore never tested before it shipped broken. + +func rowFor(rows []OffsiteRestoreRow, app string) *OffsiteRestoreRow { + for i := range rows { + if rows[i].App == app { + return &rows[i] + } + } + return nil +} + +func TestOffsiteRestoreRows_TruthTable(t *testing.T) { + now := time.Now() + inv := backup.OffsiteInventory{Apps: []backup.OffsiteInventoryApp{ + {App: "calibre-web", LatestAt: now, SizeBytes: 12345}, + {App: "immich", LatestAt: now}, + {App: offboxMarkerTag}, // the marker every snapshot carries + {App: backup.SharesPseudoStack}, // shares have their own entry, not an app wizard + }} + installed := []OffboxAppRow{ + {Name: "calibre-web", DisplayName: "Calibre-Web", Slug: "cw", Enabled: false}, // toggled OFF on purpose + {Name: "bookstack", DisplayName: "BookStack", Slug: "bs", Enabled: true}, // installed, no snapshot + } + + rows, state := buildOffsiteRestoreRows(inv, nil, installed) + if state != offsiteStoreKnown { + t.Fatalf("state = %q, want known", state) + } + + // THE FINDING: a snapshot whose app is NOT installed is listed, and is restorable. + if r := rowFor(rows, "immich"); r == nil || !r.InStore || r.Installed || !r.Restorable() { + t.Errorf("immich: want listed+in-store+not-installed+restorable, got %+v", r) + } + + // THE OTHER HALF OF THE FINDING: the future-backup toggle must not gate a past backup. + r := rowFor(rows, "calibre-web") + if r == nil || !r.Restorable() { + t.Fatalf("calibre-web must be restorable even with the offsite toggle OFF, got %+v", r) + } + if r.Enabled { + t.Errorf("fixture sanity: calibre-web should be carrying Enabled=false") + } + if r.DisplayName != "Calibre-Web" || r.Slug != "cw" { + t.Errorf("installed metadata should decorate the store row, got %+v", r) + } + + // An installed app with no snapshot is SHOWN, not silently absent — and is not restorable. + if r := rowFor(rows, "bookstack"); r == nil || r.InStore || !r.Installed || r.Restorable() { + t.Errorf("bookstack: want listed+installed+not-in-store+not-restorable, got %+v", r) + } + + // Neither marker tag is an app. + if rowFor(rows, offboxMarkerTag) != nil { + t.Errorf("the %q marker tag must never be offered as an app", offboxMarkerTag) + } + if rowFor(rows, backup.SharesPseudoStack) != nil { + t.Errorf("the shares pseudo-stack must never appear as an app row") + } +} + +// The rebuilt box: nothing installed, nothing toggled, snapshots in the store. This is the exact +// shape measured on 2026-08-06 that produced „Nincs telepített alkalmazás" and a wizard that refused +// every app. +func TestOffsiteRestoreRows_RebuiltBox_SeesItsSnapshots(t *testing.T) { + inv := backup.OffsiteInventory{Apps: []backup.OffsiteInventoryApp{ + {App: "calibre-web", LatestAt: time.Now()}, + {App: offboxMarkerTag}, + }} + rows, state := buildOffsiteRestoreRows(inv, nil, nil) // nil = NOTHING installed + if state != offsiteStoreKnown { + t.Fatalf("state = %q", state) + } + if len(rows) != 1 { + t.Fatalf("want exactly the one stored app, got %d: %+v", len(rows), rows) + } + if !rows[0].Restorable() || rows[0].Installed { + t.Errorf("a rebuilt box must be offered its stored app, got %+v", rows[0]) + } + if resolveOffsiteRestoreApp(rows, "calibre-web") == nil { + t.Errorf("the wizard must resolve an app that is in the store but not installed") + } +} + +// R-225's rule one screen over: a read failure is UNKNOWN, never empty — and it must not withhold the +// action either, because "we could not look" is not "there is nothing". +func TestOffsiteRestoreRows_UnreadableStoreIsUnknownNotEmpty(t *testing.T) { + installed := []OffboxAppRow{{Name: "calibre-web", DisplayName: "Calibre-Web"}} + rows, state := buildOffsiteRestoreRows(backup.OffsiteInventory{}, errors.New("sftp: connection refused"), installed) + if state != offsiteStoreUnreadable { + t.Fatalf("state = %q, want unreadable", state) + } + r := rowFor(rows, "calibre-web") + if r == nil || r.InStore { + t.Fatalf("unreadable store must not claim the app IS in it, got %+v", r) + } + if !r.StoreUnknown || !r.Restorable() { + t.Errorf("unreadable store must keep the action offered and flagged unknown, got %+v", r) + } + if resolveOffsiteRestoreApp(rows, "calibre-web") == nil { + t.Errorf("the wizard must not pre-emptively refuse when the store could not be read") + } +} + +// The pristine rebuilt shape (R-236): no target yet. Distinguished from a read failure because it +// resolves by itself, and the page says so. +func TestOffsiteRestoreRows_NoTargetIsItsOwnState(t *testing.T) { + _, state := buildOffsiteRestoreRows(backup.OffsiteInventory{}, backup.ErrNoOffsiteTargetSentinel(), nil) + if state != offsiteStoreNoTarget { + t.Fatalf("state = %q, want no-target", state) + } +} + +// A genuinely empty repository is empty — the one case where "nothing to restore" is true. +func TestOffsiteRestoreRows_EmptyStoreIsEmpty(t *testing.T) { + rows, state := buildOffsiteRestoreRows(backup.OffsiteInventory{Empty: true}, nil, nil) + if state != offsiteStoreKnown || len(rows) != 0 { + t.Fatalf("want known+empty, got state=%q rows=%+v", state, rows) + } +} + +// The rendered page, not just the builder: a rebuilt box must SEE its restore entry. A helper-level +// assertion could not observe this — the defect was in what the template iterated. +func TestOffsiteRestoreList_RenderedForRebuiltBox(t *testing.T) { + data := splitTestData() + data["OffboxApps"] = []OffboxAppRow{} // nothing installed + data["OffboxToggledCount"] = 0 // nothing toggled + data["OffsiteRestoreRows"] = []OffsiteRestoreRow{ + {App: "calibre-web", DisplayName: "Calibre-Web", InStore: true, Installed: false}, + } + html := renderBackupPage(t, "backups_restore", data) + + if !strings.Contains(html, `href="/backups/restore/app?name=calibre-web"`) { + t.Errorf("a rebuilt box must be offered its stored app; the page did not render the entry") + } + if !strings.Contains(html, "Nincs telepítve — a visszaállítás előbb újratelepíti.") { + t.Errorf("the not-installed consequence must be stated, not hidden") + } + if strings.Contains(html, "Nincs távoli mentésre jelölt alkalmazás") { + t.Errorf("the old toggle-keyed dead end is still rendered") + } +} + +func TestOffsiteRestoreList_RenderedStates(t *testing.T) { + for _, tc := range []struct{ state, want string }{ + {"unreadable", "nem tudjuk, mi van benne"}, + {"no-target", "kapcsolódási adatai még nem érkeztek meg"}, + } { + data := splitTestData() + data["OffsiteRestoreRows"] = []OffsiteRestoreRow{} + data["OffsiteStoreState"] = tc.state + html := renderBackupPage(t, "backups_restore", data) + if !strings.Contains(html, tc.want) { + t.Errorf("state %q: page must say %q", tc.state, tc.want) + } + if strings.Contains(html, "A távoli tároló üres") { + t.Errorf("state %q must NOT be rendered as an empty store", tc.state) + } + } +} diff --git a/controller/internal/web/restore_wizard.go b/controller/internal/web/restore_wizard.go index 0c3335e..26dab22 100644 --- a/controller/internal/web/restore_wizard.go +++ b/controller/internal/web/restore_wizard.go @@ -160,23 +160,6 @@ func deriveWizardStep(in restoreWizardInput) restoreWizardView { } } -// resolveWizardApp finds the wizard's app in the offsite-toggled set — the same gating the list page -// applies. Pure, so the two refusal rows (unknown app, app present but NOT toggled for offsite) are -// table-testable without a live backup manager. -// -// An app that is not toggled has no offsite snapshot to restore FROM, so its wizard would be a page -// of controls that cannot work. Both refusals return nil and the caller redirects — a customer-visible -// URL that survives a bookmark, an app rename or a toggle being switched off must never 500. -func resolveWizardApp(rows []OffboxAppRow, name string) *OffboxAppRow { - for _, a := range rows { - if a.Name == name && a.Enabled { - cp := a - return &cp - } - } - return nil -} - // restoreOpInFlight reports whether a restore op is in flight, FOR DISPLAY. // // **Use this, not `Manager.IsRunning()`.** The Manager carries two different booleans and they are @@ -216,9 +199,19 @@ func (s *Server) backupsRestoreWizardHandler(w http.ResponseWriter, r *http.Requ return } - row := resolveWizardApp(s.buildOffboxApps(), app) + // R-237: the gate is "is it in the store", NOT "is it toggled on for future backups". The old + // resolver required the toggle, which locked a rebuilt box out of its own snapshots — measured + // live on the R-201 re-walk. A refusal here still never 500s. + rows, storeState := s.offsiteRestoreRows(r.Context()) + row := resolveOffsiteRestoreApp(rows, app) if row == nil { - offboxRedirectTo(w, r, "/backups/restore", "Ez az alkalmazás nincs távoli mentésre kijelölve.", true) + msg := "Ehhez az alkalmazáshoz nincs mentés a távoli tárolóban." + if storeState != offsiteStoreKnown { + // Never say "there is nothing" when we could not look — R-225's rule, one screen over. + msg = "Nem tudjuk elolvasni a távoli tárolót, ezért nem tudjuk, van-e benne mentés ehhez az alkalmazáshoz." + } + s.logger.Printf("[INFO] [web] restore wizard refused for %q: not restorable (store=%s)", app, storeState) + offboxRedirectTo(w, r, "/backups/restore", msg, true) return } diff --git a/controller/internal/web/restore_wizard_test.go b/controller/internal/web/restore_wizard_test.go index 900b055..7f8e0e7 100644 --- a/controller/internal/web/restore_wizard_test.go +++ b/controller/internal/web/restore_wizard_test.go @@ -113,23 +113,30 @@ func TestDeriveWizardStep_Table(t *testing.T) { // --- Group C (Scenario B error rows): refusals resolve, never 500 ---------------------------------- -func TestResolveWizardApp_Refusals(t *testing.T) { - rows := []OffboxAppRow{ - {Name: "immich", DisplayName: "Immich", Enabled: true}, - {Name: "radarr", DisplayName: "Radarr", Enabled: false}, +func TestResolveOffsiteRestoreApp_Refusals(t *testing.T) { + // R-237 replaced the toggle gate with a store gate. The row that used to be refused for being + // untoggled (radarr) now resolves when the STORE holds it — that reversal IS the fix. + rows := []OffsiteRestoreRow{ + {App: "immich", DisplayName: "Immich", InStore: true, Enabled: true}, + {App: "radarr", DisplayName: "Radarr", InStore: true, Enabled: false}, + {App: "bookstack", DisplayName: "BookStack", Installed: true}, } - if got := resolveWizardApp(rows, "immich"); got == nil || got.DisplayName != "Immich" { - t.Fatalf("toggled app must resolve, got %+v", got) + if got := resolveOffsiteRestoreApp(rows, "immich"); got == nil || got.DisplayName != "Immich" { + t.Fatalf("a stored app must resolve, got %+v", got) } - if got := resolveWizardApp(rows, "radarr"); got != nil { - t.Errorf("an app that is NOT toggled for offsite has no snapshot to restore from — want nil, got %+v", got) + if got := resolveOffsiteRestoreApp(rows, "radarr"); got == nil { + t.Errorf("the offsite TOGGLE must not gate a PAST backup — want resolved, got nil") } - if got := resolveWizardApp(rows, "does-not-exist"); got != nil { + if got := resolveOffsiteRestoreApp(rows, "bookstack"); got != nil { + t.Errorf("an app with nothing in the store has no restore to offer — want nil, got %+v", got) + } + if got := resolveOffsiteRestoreApp(rows, "does-not-exist"); got != nil { t.Errorf("unknown app must not resolve, got %+v", got) } - if got := resolveWizardApp(nil, "immich"); got != nil { + if got := resolveOffsiteRestoreApp(nil, "immich"); got != nil { t.Errorf("empty set must not resolve, got %+v", got) } + } // The customer-visible URL must redirect, not 500, when the offsite target is not configured at all. diff --git a/controller/internal/web/templates/backups_restore.html b/controller/internal/web/templates/backups_restore.html index 921141c..e00c9b9 100644 --- a/controller/internal/web/templates/backups_restore.html +++ b/controller/internal/web/templates/backups_restore.html @@ -66,22 +66,40 @@

Visszaállítás a távoli tárolóból

Válaszd ki az alkalmazást, és a következő oldalon döntsd el, mit szeretnél: ellenőrzést külön mappába, csak a hiányzó fájlok visszahozását, vagy teljes visszaállítást. Egyik sem indul el kérdés nélkül.

- {{if .OffboxToggledCount}} + + {{if eq .OffsiteStoreState "unreadable"}} +

Nem tudjuk elolvasni a távoli tárolót, ezért nem tudjuk, mi van benne. Ez nem azt jelenti, hogy üres — próbáld újra később, vagy jelezd az üzemeltetőnek.

+ {{else if eq .OffsiteStoreState "no-target"}} +

A távoli tároló kapcsolódási adatai még nem érkeztek meg ehhez a géphez, ezért még nem tudjuk megmutatni, mi van benne. Ez magától rendeződik.

+ {{end}} + {{if .OffsiteRestoreRows}}
- {{range .OffboxApps}} - {{if .Enabled}} + {{range .OffsiteRestoreRows}} {{template "app_list_row" dict "Slug" .Slug "Name" .DisplayName}} - - Visszaállítás… + {{if .Restorable}} + {{if and .InStore (not .Installed)}} + Nincs telepítve — a visszaállítás előbb újratelepíti. + {{end}} + {{if .StoreUnknown}} + Nem tudjuk, van-e mentése — a tárolót nem sikerült elolvasni. + {{end}} + + Visszaállítás… + {{else}} + + Nincs mentése a távoli tárolóban — nincs mit visszaállítani. + {{end}} {{template "app_list_row_end"}} {{end}} - {{end}}
- {{else}} -

Nincs távoli mentésre jelölt alkalmazás — a kijelölés a Távoli mentés oldalon történik.

+ {{else if eq .OffsiteStoreState "known"}} +

A távoli tároló üres — nincs mit visszaállítani.

{{end}}