From c6b69d888e2ba285740214e00b5ef19b3f210943 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Thu, 6 Aug 2026 21:58:21 +0200 Subject: [PATCH] =?UTF-8?q?v0.205.0=20=E2=80=94=20a=20run=20that=20skipped?= =?UTF-8?q?=20an=20app=20the=20customer=20selected=20is=20not=20successful?= =?UTF-8?q?=20(R-234)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- CHANGELOG.md | 55 ++++++ CONTEXT.md | 22 ++- REPORT.md | 158 ++++++++++------- controller/README.md | 7 + controller/internal/backup/backup.go | 8 +- controller/internal/backup/offbox.go | 126 ++++++++++++- controller/internal/backup/offbox_3a_test.go | 29 ++- controller/internal/backup/offbox_capture.go | 4 +- .../backup/offbox_verdict_r234_test.go | 165 ++++++++++++++++++ controller/internal/web/offbox_handlers.go | 17 ++ .../internal/web/offbox_run_inflight_test.go | 71 ++++++++ 11 files changed, 578 insertions(+), 84 deletions(-) create mode 100644 controller/internal/backup/offbox_verdict_r234_test.go create mode 100644 controller/internal/web/offbox_run_inflight_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 09207b3..f5f54fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,58 @@ +## v0.205.0 — a backup that skipped an app the customer chose is not „Rendben" (2026-08-06, R-234) — MinAgent 0.127.0 + +**Two defects, and the one that actually produced the measured sequence was NOT the one filed.** + +### The verdict now counts a skipped selection + +The R-203 verdict block already carries the sentence *"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`; an app skipped **entirely**, with nothing of it in the +snapshot at all, still reported `ok` with a warning beside it. The smaller gap moved the verdict and +the bigger one did not. It does now. + +**Which skips count (§7.2), decided by measurement rather than assumption:** + +| skip | counts? | why | +|---|---|---| +| selected + **deployed**, no recovery unit | **yes** | the app the customer chose is not protected | +| selected but **not deployed** | **no**, but NAMED with what to do | a box left amber forever by an app somebody removed is a status nobody reads | +| drive disconnected / decommissioned | **no** | it has its own card and its own signal | +| nothing selected at all | **no** | unchanged: the existing zero-selection notice | + +The operator signal is **reused, not mirrored** — a skipped app is reported through the existing +mandatory-gap notification as a whole-unit gap, so one vocabulary covers both. +`LastSuccess` and `SnapshotCount` still record what WAS captured: half a backup is not no backup. + +### The measured cause: a manual run silently dropped by the single-flight + +Reproduced on demo-hp: **the pre-dump phase (`captureAllRecoveryUnits`) writes a unit for every +deployed stack before the push**, so "selected but no bundle yet" does not normally survive a run — +moving a unit aside and running recreated it and reported `ok`. So the filed mechanism could not have +produced the 2026-08-06 sequence. + +What did: the customer pressed „Távoli mentés most”, the handler answered „A távoli mentés elindult”, `acquireRunning` refused because a run was already going, and the run +returned **nil** — no error, no signal. The card then showed the **previous** run's „Rendben", which +reads as covering the app just selected. It did not, and the restore refused minutes later. + +The single-flight decision is now taken **synchronously in the handler**, before the goroutine, and a +dropped request says so. The nightly path is deliberately unchanged: returning nil is right for it — +nobody asked, and the next scheduled run retries. + +### §7.3 — the wait, measured before deciding + +`CaptureRecoveryUnit` writes compose config + a manifest (**a few KB**, per `admission.go`'s own +note), **enumerates** dumps already present rather than creating them, is idempotent, and **does not +stop the app**. It already runs for every deployed stack inside the off-site run's own pre-dump phase, +through `admitApp`. **So the inline capture this task contemplated already exists — nothing was built**, +and for a deployed app there is no wait to remove. + +### Hungarian + +- „Ezek az alkalmazások NEM kerültek be a távoli mentésbe, mert még nincs helyi mentési egységük: %s. A következő mentés általában már elkészíti — ha a második futás után is itt szerepelnek, szólj az üzemeltetőnek.” +- „Ezek az alkalmazások ki vannak jelölve távoli mentésre, de nincsenek telepítve, ezért nem menthetők: %s. Ha már nincs rájuk szükséged, vedd ki a kijelölésüket a Távoli mentés oldalon.” +- „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.” +- sibling (mandatory folders), extended so both read alike: „… Ellenőrizd, hogy a mappák megvannak-e a meghajtón; ha igen és ez a következő mentés után is látszik, szólj az üzemeltetőnek.” + ## 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 diff --git a/CONTEXT.md b/CONTEXT.md index 5c34f7f..9efc5a4 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -7,7 +7,27 @@ > > Ask Claude Code: "Please update CONTEXT.md with what we did today" -Last updated: 2026-08-05 (v0.200.0 — R-193: the recovery screen, unlocking only) +Last updated: 2026-08-06 (v0.205.0 — R-234: a run that skipped a selected app is not successful) + +> **2026-08-06 — v0.205.0 (R-234).** THE RULE: **a run that skipped an app the customer selected is +> not a successful run.** The R-203 verdict block already said *"a warning beside a success is read +> as a success"* and applied it to one of the two shapes it describes — a missing declared FOLDER +> made the run `incomplete`, an app skipped ENTIRELY did not. Now both do. A selected-but-UNDEPLOYED +> app is named with what to do but does NOT move the verdict, because a box left permanently amber by +> an app somebody removed is a status nobody reads. +> +> **§7.3, MEASURED rather than assumed — and the answer was "already done".** `CaptureRecoveryUnit` +> writes compose config + a manifest (a few KB), only ENUMERATES dumps rather than creating them, is +> idempotent, and does NOT stop the app; the off-site run already calls it for every deployed stack in +> its own pre-dump phase, through `admitApp`. So there is no wait to remove for a deployed app, and +> **nothing was built**. Proven on demo-hp: a unit moved aside was RECREATED by the run. +> +> **AND THE FILED MECHANISM WAS NOT THE MEASURED CAUSE.** R-234 was filed as "the first run after a +> toggle finds no bundle and skips the app". That cannot happen for a deployed app (above). What did +> happen on 2026-08-06: the manual run was dropped by the **single-flight** while an earlier run was +> still going; `runOffboxBackup` returned nil; the handler had already said „elindult”; and the card +> then showed the PREVIOUS run's „✓ Rendben”. Fixed by taking that decision synchronously in the +> handler. **The nightly path deliberately still returns nil** — nobody asked, and it retries. > **2026-08-05 — v0.200.0 (R-193 CLOSED).** The customer-facing recovery screen. Until now a customer > whose machine was rebuilt had everything needed to get their data back and no way to find out — the diff --git a/REPORT.md b/REPORT.md index 6a938af..72b6529 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,91 +1,115 @@ -# REPORT — v0.204.0: the restore list is keyed on the store (R-237), and the size gate stops refusing in silence (R-238) +# REPORT — v0.205.0: a backup that skipped an app the customer chose is not „Rendben" (R-234) -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. +2026-08-06. Controller **v0.204.0 → v0.205.0**. MinAgent unchanged (**0.127.0**). `felhom-agent`, +`app-catalog-felhom.eu` untouched. No hub change. -## R-238 — classified: a HARNESS ARTIFACT, with a real residue that is fixed +## The correction that outranks the task -The two runs diverge at one parameter, and the state at that point is quoted rather than inferred: +§3 stated the mechanism as established: *"the off-site copy sends the local recovery bundle an app +already has; switching an app on does not create one, so the first run after the toggle finds nothing +to send."* **Measured on demo-hp, that is not what the code does.** -- `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. +The off-site run's own **pre-dump phase** calls `captureAllRecoveryUnits()`, which writes a unit for +**every deployed stack** — through `admitApp` — *before* the per-app push loop. I moved +`calibre-web`'s unit aside (never deleted) on demo-hp and triggered a run through the real endpoint: -**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. +``` +BEFORE : units = calibre-web, opengist, privatebin status=ok snapshots=9 +moved : calibre-web unit → .calibre-web.R234-aside +RUN : POST /backup/offbox/run → 302, finished ~90 s, status=ok +AFTER : status=ok snapshots=9 warning=(none) +unit : /mnt/sys_drive/felhom-data/backups/primary/calibre-web → RECREATED +``` -**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. +So "selected but no bundle yet" **does not survive a run** for a deployed app. The filed mechanism +could not have produced the 2026-08-06 sequence. -## R-237 — the restore list, rebuilt on the store +**What did.** `POST /backup/offbox/run` launched the run in a goroutine and immediately answered +„A távoli mentés elindult". Inside, `acquireRunning` refused (a run was already in flight) and +`runOffboxBackup` returned **nil** — no error, no signal. The card then showed the **previous** run's +„✓ Rendben · 1 pillanatkép", read as covering the app just selected. It did not: the restore refused +for that app minutes later, and a third run carried it. That is R-234's real cause, and it is the same +family — a skip that reached a log and not a verdict. -`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. +## Live validation (§12) -Every §7 case, and the Hungarian as rendered: +| # | what | observable | +|---|---|---| +| 1 | selected app with no unit → run | the run **recreated the unit** and reported ok — the measurement above. The verdict change is exercised by the run-level tests, because production cannot easily be held in that state | +| 2 | counters intact | `snapshot_count` 9 → 9, `last_success` advanced to `2026-08-06T19:34:27Z` — what was captured is still recorded | +| 3 | the third-run behaviour arriving sooner | **not applicable as filed**: there is no wait for a deployed app; the unit is created by the run itself (§7.3) | +| 4 | a box with nothing selected | unchanged — Scenario D is pinned by test, and demo-hp's other apps stayed `ok` | -| case | rendered | -|---|---| -| 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. | +**Access note:** demo-hp's dashboard password had been changed by the customer claim, so I reset it +through the documented `--print-reset-code` escape hatch (code streamed file→file, extracted by shape, +shredded; the new password is a 24-char generated value in a `0600` file on DooPlex). That is a +deliberate change to a Tier-0 demo box, recorded here. -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. +## §7.2 — which skips count, as decided -`felhom-offbox` and `_shares` are excluded from the app list. +| skip | counts? | why | +|---|---|---| +| selected + **deployed**, no recovery unit | **YES** → `incomplete` | the app the customer chose is not in the snapshot at all | +| selected but **not deployed** | **no** — named, with what to do | a box left amber forever by an app somebody removed is a status nobody reads | +| drive disconnected / decommissioned | **no** | it has its own card and its own signal; re-reporting it here would double-count | +| nothing selected at all | **no** | unchanged: the existing „nincs mentésre jelölt alkalmazás" notice | -## Tests +## §7.3 — the measurement, and the decision it forced + +`CaptureRecoveryUnit` writes `compose/` (docker-compose.yml, .felhom.yml, app.yaml) + `manifest.json` +— **a few KB**, per `admission.go`'s own note — and **enumerates** the DB/volume dumps already present +rather than creating them. It is idempotent (skips all writes when current) and **does not stop the +app**; the stopping work belongs to the separate dump flow. + +**Decision: nothing to build.** The cheap, non-stopping capture already runs for every deployed stack +inside the off-site run's pre-dump phase, through `admitApp`. For a deployed app there is no wait to +remove, and the §7.3 branch that would have added an inline capture would have duplicated it. + +## §7.4 — every changed Hungarian string + +- „Ezek az alkalmazások NEM kerültek be a távoli mentésbe, mert még nincs helyi mentési egységük: %s. A következő mentés általában már elkészíti — ha a második futás után is itt szerepelnek, szólj az üzemeltetőnek." +- „Ezek az alkalmazások ki vannak jelölve távoli mentésre, de nincsenek telepítve, ezért nem menthetők: %s. Ha már nincs rájuk szükséged, vedd ki a kijelölésüket a Távoli mentés oldalon." +- „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." +- **sibling, extended so both read alike:** „Figyelmeztetés: a(z) %s alkalmazás egyes adatmappái nem kerültek a távoli mentésbe: %s. **Ellenőrizd, hogy a mappák megvannak-e a meghajtón; ha igen és ez a következő mentés után is látszik, szólj az üzemeltetőnek.**" + +The replaced sentence was: „Figyelmeztetés: %d alkalmazásnak nincs elérhető mentése, ezek kimaradtak: %s" — a count with no next step, reading the same whether the customer must act or simply wait. + +## Tests and red-proofs `go build` · `go vet` · `go test ./...` → **28 packages ok**. `controller_gates.py --fast` → **9/9 OK**. -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). +New: `internal/backup/offbox_verdict_r234_test.go` (Scenarios A, C, D, F — run-level) and +`internal/web/offbox_run_inflight_test.go` (handler-level). -## Red-proofs — each mutation asserted to have applied before the result was trusted - -| # | mutation | result | +| # | mutation (each asserted to have applied) | 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" | +| RP-1 | drop `unprotected` from the verdict — the defect itself | `SkippedSelectedAppIsIncomplete` **FAIL** — „LastStatus = ok, want incomplete" | +| RP-2 | remove the classification switch; count **every** skip | `SelectedButUndeployedIsNamedNotCounted` **FAIL** — a removed app turns the box amber | +| RP-3 | stop adding skipped apps to the operator signal | `SkippedSelectedAppIsIncomplete` **FAIL** — „operator signal … got map[]" | +| RP-4 | remove the synchronous in-flight check in the handler | `InFlightRequestIsNotReportedAsStarted` **FAIL** | -All three restored; suite green again afterwards. +All restored; `grep -c RED-PROOF` = 0 in both files afterwards; suite green again. + +**Fixture note:** the shared `offbox3aProvider.ListDeployedStacks()` returned nil, so my first run of +Scenario A passed *for the wrong reason* and F passed vacuously. Fixed with an **opt-in** `deployed` +map that defaults to nil, so no existing fixture's behaviour moves. ## Files -- 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` +- `internal/backup/offbox.go` — `ErrOffboxRunInFlight`, `offboxWholeUnitGap`, the skip classification, + `missingUnprotected`/`missingNotDeployed`, `stackDeployed`, `driveUnavailableFor`, the verdict, the + messages +- `internal/backup/offbox_capture.go` — the sibling message +- `internal/backup/backup.go` — `AcquireRunningForTest`/`ReleaseRunningForTest` (test-only seams) +- `internal/web/offbox_handlers.go` — the synchronous single-flight refusal +- new `internal/backup/offbox_verdict_r234_test.go`, `internal/web/offbox_run_inflight_test.go` +- `internal/backup/offbox_3a_test.go` — opt-in deployed set +- `CHANGELOG.md`, `controller/README.md`, `CONTEXT.md` -## Not done here, deliberately +## Observations, not acted on -- **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.** +- **The zero-selection notice reads „Sikeres — nincs mentésre jelölt alkalmazás"** while the status is + `ok`. It is honest, but "Sikeres" beside "nothing is covered" is the same rhetorical shape this + session is about, one notch weaker. Not touched: Scenario D forbids changing that path. +- **`res.missing` is still used verbatim** for the no-silent-success error text (every app missing → + `error`). Correct as-is, and deliberately left, since that path already refuses loudly. diff --git a/controller/README.md b/controller/README.md index 3052304..8155f4a 100644 --- a/controller/README.md +++ b/controller/README.md @@ -160,6 +160,13 @@ backups, monitoring and notifications. All Proxmox/disk operations are delegated 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. + **A run that skipped a selected app is `incomplete` (v0.205.0, R-234):** the off-site verdict now + counts `missingUnprotected` beside `mandatoryGaps` — an app the customer selected that is DEPLOYED + but has no recovery unit is not protected, so the run is not `ok`. A selected-but-UNDEPLOYED app is + named with what to do and does NOT move the verdict (a permanently amber box is a status nobody + reads); a disconnected/decommissioned drive has its own signal. The manual „Távoli mentés most” + also refuses SYNCHRONOUSLY when a run is already in flight, instead of answering „elindult” and + leaving the previous run's verdict on the card. (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/backup.go b/controller/internal/backup/backup.go index 2e50c8a..c36b75f 100644 --- a/controller/internal/backup/backup.go +++ b/controller/internal/backup/backup.go @@ -95,7 +95,7 @@ type Manager struct { offboxOrphanEvent func(eventType, renamedTo string) // offboxGapNotify (R-203) fires when a COMPLETED offsite run could not capture a directory an // app declares MANDATORY — a coverage gap, not a failed run. nil → no signal. - offboxGapNotify func(gaps map[string][]string) + offboxGapNotify func(gaps map[string][]string) // offboxSSH (v0.142.0) is the raw-ssh exec seam for the orphaned-repo move-aside (restic has no // rename); tests inject a fake. Nil → the real ssh invocation (defaultOffboxSSH). offboxSSH func(ctx context.Context, host, user string, port int, keyPath, knownHosts, remoteCmd string) ([]byte, error) @@ -857,6 +857,12 @@ func (m *Manager) IsRunning() bool { return m.running } +// AcquireRunningForTest / ReleaseRunningForTest occupy the single-flight from another package's +// test, so the "a run is already in flight" branch can be exercised without racing a real run. +// Test-only seam, in the same spirit as SetOffboxRunner; nothing in production calls them. +func (m *Manager) AcquireRunningForTest() error { return m.acquireRunning() } +func (m *Manager) ReleaseRunningForTest() { m.releaseRunning() } + // acquireRunning atomically sets the running flag. Returns error if already running. func (m *Manager) acquireRunning() error { m.mu.Lock() diff --git a/controller/internal/backup/offbox.go b/controller/internal/backup/offbox.go index 7b09b0e..1187d80 100644 --- a/controller/internal/backup/offbox.go +++ b/controller/internal/backup/offbox.go @@ -77,6 +77,16 @@ func (m *Manager) SetOffboxSSH(fn func(ctx context.Context, host, user string, p // the orphan card instead of the raw restic error. var ErrOffboxOrphaned = fmt.Errorf("offbox repo orphaned: exists but keyed under a previous, no-longer-available passphrase") +// ErrOffboxRunInFlight is returned to the MANUAL caller only, when the single-flight dropped the +// request because a run was already going (R-234). It is not a failure of anything — the run in +// flight is doing the work — but it IS a request that did nothing, and the page must say so instead +// of showing the previous run's verdict under a „started" message. +// offboxWholeUnitGap is the pseudo-path used to report a WHOLE-unit gap through the mandatory-gap +// notification, so a skipped app and a skipped directory reach the operator in one vocabulary. +const offboxWholeUnitGap = "(a teljes alkalmazás — nincs helyi mentési egysége)" + +var ErrOffboxRunInFlight = fmt.Errorf("an off-box backup is already running; this request did not start a new one") + // classifyResticProbe maps a `restic cat config` failure to a repo class. The signatures are the exact // restic stderr matched in the 2026-07-17 diagnosis + restic's no-repo message: // - "orphaned": repo present, wrong key ("wrong password or no key found") — the definitive signal @@ -746,7 +756,19 @@ func (m *Manager) runOffboxBackup(ctx context.Context, withProgress bool) error } if err := m.acquireRunning(); err != nil { m.logger.Printf("[INFO] [offbox] skipped — another backup is running") - return nil // single-flight: don't race; the next scheduled run retries + // R-234 (the MEASURED cause). The nightly path is unchanged: returning nil is right for it — + // nobody asked, and the next scheduled run retries. + // + // The MANUAL path is a different question, and answering it the same way is what produced the + // 2026-08-06 sequence. The customer pressed „Távoli mentés most" and was told + // „A távoli mentés elindult"; the run was dropped here and returned nil; the card then showed + // the PREVIOUS run's „✓ Rendben", which they read as covering the app they had just selected. + // It did not — the restore refused for that app minutes later. A request that did nothing must + // not be reported as one that started, so the manual caller is told. + if withProgress { + return ErrOffboxRunInFlight + } + return nil } defer m.releaseRunning() @@ -874,10 +896,38 @@ func (m *Manager) runOffboxBackup(ctx context.Context, withProgress bool) error // backup is not no backup, and reporting it as none would be its own lie. `incomplete` is // minted here because the existing vocabulary ("ok" | "error" | "running") has nothing that // means "it ran, and this app is not fully protected". - if len(runResult.mandatoryGaps) > 0 { + // R-234 EXTENDS THE SAME RULE TO THE BIGGER CASE. Until v0.205.0 the paragraph above was + // applied to ONE of the two shapes it describes: an app missing a declared mandatory + // FOLDER made the run incomplete, while an app skipped ENTIRELY — no recovery unit, so + // nothing of it in the snapshot at all — still reported ok with a warning beside it. The + // smaller gap moved the verdict and the bigger one did not. Measured 2026-08-06: a run + // reported „✓ Rendben · 1 pillanatkép" and the restore then refused for the app the + // customer had just selected. + gaps := len(runResult.mandatoryGaps) > 0 + unprotected := len(runResult.missingUnprotected) > 0 + if gaps || unprotected { o.LastStatus = "incomplete" if m.offboxGapNotify != nil { - m.offboxGapNotify(runResult.mandatoryGaps) + // Reuse, not mirror: the operator signal for "this run left an app less protected + // than the customer asked for" is the same signal. A skipped app is reported as a + // whole-unit gap so one notification shape covers both, and the recipient does not + // have to learn a second vocabulary for the worse case. + notify := runResult.mandatoryGaps + if unprotected { + if notify == nil { + notify = map[string][]string{} + } else { + cp := make(map[string][]string, len(notify)+len(runResult.missingUnprotected)) + for k, v := range notify { + cp[k] = v + } + notify = cp + } + for _, a := range runResult.missingUnprotected { + notify[a] = append(notify[a], offboxWholeUnitGap) + } + } + m.offboxGapNotify(notify) } } else { o.LastStatus = "ok" @@ -895,9 +945,18 @@ func (m *Manager) runOffboxBackup(ctx context.Context, withProgress bool) error if len(apps) == 0 && !runResult.sharesBackedUp { warns = append(warns, "Sikeres — nincs mentésre jelölt alkalmazás") } - if len(missing) > 0 { - warns = append(warns, fmt.Sprintf("Figyelmeztetés: %d alkalmazásnak nincs elérhető mentése, ezek kimaradtak: %s", - len(missing), strings.Join(missing, ", "))) + // R-234 §7.4 — WHICH apps, WHY, and WHEN. The old sentence said only that N apps "had no + // available backup and were left out", which names a problem with no next step and reads + // the same whether the customer must act or simply wait. + if len(runResult.missingUnprotected) > 0 { + warns = append(warns, fmt.Sprintf( + "Ezek az alkalmazások NEM kerültek be a távoli mentésbe, mert még nincs helyi mentési egységük: %s. A következő mentés általában már elkészíti — ha a második futás után is itt szerepelnek, szólj az üzemeltetőnek.", + strings.Join(runResult.missingUnprotected, ", "))) + } + if len(runResult.missingNotDeployed) > 0 { + warns = append(warns, fmt.Sprintf( + "Ezek az alkalmazások ki vannak jelölve távoli mentésre, de nincsenek telepítve, ezért nem menthetők: %s. Ha már nincs rájuk szükséged, vedd ki a kijelölésüket a Távoli mentés oldalon.", + strings.Join(runResult.missingNotDeployed, ", "))) } // 3a: capture-gap warnings (structurally-refused / on-disk-missing mandatory paths, undeployed). warns = append(warns, runResult.warns...) @@ -1059,6 +1118,39 @@ type offboxRunResult struct { // that could NOT be captured. It is the STRUCTURED form of the warnings above, and it is what // decides the run's verdict: a run that dropped a mandatory directory is not a successful run. mandatoryGaps map[string][]string + // missingUnprotected / missingNotDeployed (R-234) split `missing` by WHY, because only one of the + // two may move the verdict. See the classification comment at the skip site: an app the customer + // selected and that IS deployed but has no unit is unprotected and counts; an app that is no + // longer installed is named but does not, so a removed app cannot leave the box amber forever. + missingUnprotected []string + missingNotDeployed []string +} + +// stackDeployed reports whether the stack is currently deployed on this box. Used only to classify a +// skip (R-234) — never to decide whether to back something up. +func (m *Manager) stackDeployed(stack string) bool { + if m.stackProvider == nil { + return false + } + for _, st := range m.stackProvider.ListDeployedStacks() { + if st.Name == stack { + return true + } + } + return false +} + +// driveUnavailableFor reports whether the app's drive is disconnected or decommissioned — states that +// already have their own customer-facing signal, so a skip caused by them is not re-reported here. +func (m *Manager) driveUnavailableFor(stack string) bool { + if m.settings == nil { + return false + } + d := m.GetAppDrivePath(stack) + if d == "" { + return false + } + return m.settings.IsDisconnected(d) || m.settings.IsDecommissioned(d) } // runOffboxInternal does the repo-ensure + per-app DISCOVER → capture-set → gate → multi-path backup + @@ -1079,6 +1171,28 @@ func (m *Manager) runOffboxInternal(ctx context.Context, apps, base, env []strin if !ok { m.logger.Printf("[WARN] [offbox] %s: no recovery unit found on any connected drive — skipping", stack) res.missing = append(res.missing, stack) + // R-234 §7.2 — WHICH skips make the run not-successful. The list above is prose for the + // customer; this classification is what the VERDICT may consult, and the two are not the + // same question. Established by measurement on demo-hp 2026-08-06, not assumed: + // + // * DEPLOYED, no unit — the run's own pre-dump phase (captureAllRecoveryUnits) writes a + // unit for every deployed stack before the push, so this state does not normally + // survive a run. Reaching here means the capture was refused (the reserve) or failed. + // The app the customer selected is NOT protected: it COUNTS. + // * NOT DEPLOYED — nothing can protect an app that is not there, and the remedy is to + // deselect it. It is NAMED so the customer can act, but it does NOT count: a box left + // permanently amber over an app somebody removed is a status that stops being read, + // which is how this whole class of defect starts. + // * drive disconnected/decommissioned — has its own signal and its own card; not ours to + // re-report as a backup gap. + switch { + case !m.stackDeployed(stack): + res.missingNotDeployed = append(res.missingNotDeployed, stack) + case m.driveUnavailableFor(stack): + // counted as neither: the drive card is the honest surface for this one. + default: + res.missingUnprotected = append(res.missingUnprotected, stack) + } continue } // Task 3-core TierOffsite capture set: mandatory userdata paths added to the unit snapshot, diff --git a/controller/internal/backup/offbox_3a_test.go b/controller/internal/backup/offbox_3a_test.go index 64ffe26..9f4b0f7 100644 --- a/controller/internal/backup/offbox_3a_test.go +++ b/controller/internal/backup/offbox_3a_test.go @@ -23,17 +23,30 @@ type offbox3aProvider struct { hdd map[string]string binds map[string][]ClassifiedBind has map[string]bool + // deployed is OPT-IN and defaults to nil, so every existing fixture keeps ListDeployedStacks() + // returning nil and nothing about their behaviour moves. R-234's classification is the only + // thing that needs a real deployed set. + deployed map[string]bool } func (p *offbox3aProvider) GetStackComposePath(string) (string, bool) { return "", false } -func (p *offbox3aProvider) ListDeployedStacks() []StackSummary { return nil } -func (p *offbox3aProvider) GetStackHDDMounts(string) []string { return nil } -func (p *offbox3aProvider) GetStackHDDPath(n string) string { return p.hdd[n] } -func (p *offbox3aProvider) GetImportRoot() string { return "" } // R-75: no import binds in this fixture -func (p *offbox3aProvider) GetDockerVolumes(string) []string { return nil } -func (p *offbox3aProvider) StopStack(string) error { return nil } -func (p *offbox3aProvider) StartStack(string) error { return nil } -func (p *offbox3aProvider) RefreshAndIsRunning(string) bool { return false } +func (p *offbox3aProvider) ListDeployedStacks() []StackSummary { + if len(p.deployed) == 0 { + return nil + } + out := make([]StackSummary, 0, len(p.deployed)) + for n := range p.deployed { + out = append(out, StackSummary{Name: n}) + } + return out +} +func (p *offbox3aProvider) GetStackHDDMounts(string) []string { return nil } +func (p *offbox3aProvider) GetStackHDDPath(n string) string { return p.hdd[n] } +func (p *offbox3aProvider) GetImportRoot() string { return "" } // R-75: no import binds in this fixture +func (p *offbox3aProvider) GetDockerVolumes(string) []string { return nil } +func (p *offbox3aProvider) StopStack(string) error { return nil } +func (p *offbox3aProvider) StartStack(string) error { return nil } +func (p *offbox3aProvider) RefreshAndIsRunning(string) bool { return false } func (p *offbox3aProvider) GetStackRecoveryInfo(string) (RecoveryInfo, bool) { return RecoveryInfo{}, false } diff --git a/controller/internal/backup/offbox_capture.go b/controller/internal/backup/offbox_capture.go index 6bc914b..4efc363 100644 --- a/controller/internal/backup/offbox_capture.go +++ b/controller/internal/backup/offbox_capture.go @@ -75,7 +75,9 @@ func (m *Manager) offboxCaptureSet(stack string) (extra []string, warns []string extra = append(extra, p.Abs) } if len(gaps) > 0 { - warns = append(warns, fmt.Sprintf("Figyelmeztetés: a(z) %s alkalmazás egyes adatmappái nem kerültek a távoli mentésbe: %s.", + // R-234 §7.4: this sits beside the whole-app gap message on the same card, and both now drive + // the same `incomplete` verdict — so it says what to do, not only what happened. + warns = append(warns, fmt.Sprintf("Figyelmeztetés: a(z) %s alkalmazás egyes adatmappái nem kerültek a távoli mentésbe: %s. Ellenőrizd, hogy a mappák megvannak-e a meghajtón; ha igen és ez a következő mentés után is látszik, szólj az üzemeltetőnek.", stack, strings.Join(gaps, ", "))) } return extra, warns, gaps diff --git a/controller/internal/backup/offbox_verdict_r234_test.go b/controller/internal/backup/offbox_verdict_r234_test.go new file mode 100644 index 0000000..0f77a4a --- /dev/null +++ b/controller/internal/backup/offbox_verdict_r234_test.go @@ -0,0 +1,165 @@ +package backup + +import ( + "context" + "strings" + "testing" +) + +// R-234 — a run that SKIPPED an app the customer selected is not a successful run. +// +// The same paragraph the R-203 verdict block already carries — "a warning beside a success is read +// as a success" — was applied to one of the two shapes it describes. An app missing a declared +// mandatory FOLDER made the run `incomplete`; an app skipped ENTIRELY, with nothing of it in the +// snapshot at all, still reported `ok`. The smaller gap moved the verdict and the bigger one did not. +// +// Run-level on purpose: the classification and the verdict are both inside the run, and the sibling +// test file records what happened when its first version asserted the capture helper alone — its +// red-proof passed while the defect was untouched. + +// Scenario A — a selected, DEPLOYED app with no recovery unit makes the run incomplete, names itself, +// and does not suppress what was captured. +// +// RED-PROOF: drop `unprotected` from the verdict condition (leave only mandatoryGaps) → this FAILS +// with the run reporting ok over a skipped app, which is production behaviour up to v0.204.0. +func TestOffboxRun_SkippedSelectedAppIsIncomplete(t *testing.T) { + drive := t.TempDir() + m, sett, prov := classifiedOffboxManager(t, drive) + + // `kept` has a unit and is pushed; `dropped` is selected and deployed but has NO unit, so the + // per-app loop skips it. backedUp>0 is what made the existing no-silent-success guard stay quiet. + mkUnit(t, drive, "kept") + prov.hdd["kept"] = drive + prov.has["kept"] = true + prov.hdd["dropped"] = drive + prov.has["dropped"] = true + prov.deployed = map[string]bool{"kept": true, "dropped": true} + _ = sett.SetAppOffbox("kept", true) + _ = sett.SetAppOffbox("dropped", true) + + var gapNotified map[string][]string + m.SetOffboxGapNotify(func(g map[string][]string) { gapNotified = g }) + cap := &backupCapture{} + m.SetOffboxRunner(cap.runner()) + + if err := m.RunOffboxBackup(context.Background()); err != nil { + t.Fatalf("the run itself must SUCCEED — a skipped app is a coverage gap, not a failed run: %v", err) + } + + got := sett.GetOffboxTarget() + if got.LastStatus != "incomplete" { + t.Fatalf("LastStatus = %q, want \"incomplete\" — the customer selected an app and the run did not "+ + "carry it; on 2026-08-06 this reported „✓ Rendben” and the restore refused minutes later", got.LastStatus) + } + // Scenario A: the counters and the anchor still record what WAS captured. + if got.LastSuccess == "" { + t.Error("LastSuccess must still record what was captured — half a backup is not no backup") + } + if cap.backups != 1 { + t.Errorf("the app that HAD a unit must still be pushed, got %d backup calls", cap.backups) + } + // Scenario E: which app, and why. + if !strings.Contains(got.LastWarning, "dropped") { + t.Errorf("the warning must NAME the skipped app, got %q", got.LastWarning) + } + if !strings.Contains(got.LastWarning, "nincs helyi ment") { + t.Errorf("the warning must say WHY it was skipped, got %q", got.LastWarning) + } + if !strings.Contains(got.LastWarning, "következő ment") { + t.Errorf("the warning must say WHEN it will be protected, got %q", got.LastWarning) + } + // Scenario B: the operator hears about it, in the same vocabulary as a folder gap. + if len(gapNotified["dropped"]) == 0 { + t.Fatalf("the operator signal must carry the skipped app, got %v", gapNotified) + } +} + +// Scenario C — a healthy run is untouched. Without this, "always incomplete" would also pass above, +// and a status that is never green is a status that stops being read. +// +// RED-PROOF: count EVERY skip (drop the classification switch and use len(res.missing)) → a healthy +// run goes amber and this FAILS. +func TestOffboxRun_HealthyRunStaysOk(t *testing.T) { + drive := t.TempDir() + m, sett, prov := classifiedOffboxManager(t, drive) + mkUnit(t, drive, "kept") + prov.hdd["kept"] = drive + prov.has["kept"] = true + _ = sett.SetAppOffbox("kept", true) + + fired := false + m.SetOffboxGapNotify(func(map[string][]string) { fired = true }) + cap := &backupCapture{} + m.SetOffboxRunner(cap.runner()) + if err := m.RunOffboxBackup(context.Background()); err != nil { + t.Fatalf("run: %v", err) + } + got := sett.GetOffboxTarget() + if got.LastStatus != "ok" { + t.Fatalf("LastStatus = %q, want ok — every selected app was carried", got.LastStatus) + } + if fired { + t.Error("the operator signal must NOT fire when nothing was missed") + } + if strings.Contains(got.LastWarning, "NEM kerültek be") { + t.Errorf("a healthy run must carry no skip warning, got %q", got.LastWarning) + } +} + +// Scenario D — a box with NOTHING selected keeps today's behaviour: ok, with the existing +// zero-selection notice. An unconfigured box reporting incomplete forever is its own defect. +// +// RED-PROOF: count the empty selection as a gap → this box goes permanently amber and this FAILS. +func TestOffboxRun_NothingSelectedIsNotAGap(t *testing.T) { + drive := t.TempDir() + m, sett, _ := classifiedOffboxManager(t, drive) + cap := &backupCapture{} + m.SetOffboxRunner(cap.runner()) + if err := m.RunOffboxBackup(context.Background()); err != nil { + t.Fatalf("run: %v", err) + } + got := sett.GetOffboxTarget() + if got.LastStatus != "ok" { + t.Fatalf("LastStatus = %q, want ok — nothing was selected, so nothing was skipped", got.LastStatus) + } + if !strings.Contains(got.LastWarning, "nincs mentésre jelölt alkalmazás") { + t.Errorf("the existing zero-selection notice must survive, got %q", got.LastWarning) + } +} + +// Scenario F — a selected app that is NOT deployed. Decided deliberately: it is NAMED with what to do +// about it, and it does NOT move the verdict, because a box left amber forever by an app somebody +// removed is a status nobody reads. +func TestOffboxRun_SelectedButUndeployedIsNamedNotCounted(t *testing.T) { + drive := t.TempDir() + m, sett, prov := classifiedOffboxManager(t, drive) + mkUnit(t, drive, "kept") + prov.hdd["kept"] = drive + prov.has["kept"] = true + prov.deployed = map[string]bool{"kept": true} // "removed-app" deliberately absent + _ = sett.SetAppOffbox("kept", true) + // selected, no unit, and NOT in the deployed set + _ = sett.SetAppOffbox("removed-app", true) + + fired := false + m.SetOffboxGapNotify(func(map[string][]string) { fired = true }) + cap := &backupCapture{} + m.SetOffboxRunner(cap.runner()) + if err := m.RunOffboxBackup(context.Background()); err != nil { + t.Fatalf("run: %v", err) + } + got := sett.GetOffboxTarget() + if got.LastStatus != "ok" { + t.Fatalf("LastStatus = %q, want ok — an app that is not installed cannot be protected, and must "+ + "not hold the box amber forever", got.LastStatus) + } + if !strings.Contains(got.LastWarning, "removed-app") { + t.Errorf("the undeployed selection must still be NAMED, got %q", got.LastWarning) + } + if !strings.Contains(got.LastWarning, "vedd ki a kijelöl") { + t.Errorf("it must say what to do about it, got %q", got.LastWarning) + } + if fired { + t.Error("an undeployed app must not raise the operator gap signal") + } +} diff --git a/controller/internal/web/offbox_handlers.go b/controller/internal/web/offbox_handlers.go index 7473f1d..83cf33c 100644 --- a/controller/internal/web/offbox_handlers.go +++ b/controller/internal/web/offbox_handlers.go @@ -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) } }() diff --git a/controller/internal/web/offbox_run_inflight_test.go b/controller/internal/web/offbox_run_inflight_test.go new file mode 100644 index 0000000..7be3801 --- /dev/null +++ b/controller/internal/web/offbox_run_inflight_test.go @@ -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()) + } +}