diff --git a/REPORT.md b/REPORT.md index 21805b8..466ce93 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,314 +1,277 @@ -# REPORT — R-47: the DB replay must not race the app (both restore paths) · felhom-controller v0.153.0 +# REPORT — v0.154.0 (R-48 restore wizard) + Part 2 docs; **Part 3 STOPPED**, STOP-1 pending -**Date:** 2026-07-20 · **Repo:** `felhom-controller` (v0.152.0 → **v0.153.0**) · Trunk, pushed to -`main`. · **Baseline:** `main` @ `fd40b29` (clean, equal to `origin/main` at session start) +Session date: **2026-07-21**. Executed on DooPlex as `kisfenyo`. ---- +## 0. Outcome at a glance -## 1. What was wrong - -`felhom.eu/documentation/audits/DIAG-immich-restore-round2-2026-07-19.md`, finding **H4**. The -offsite reconstitution ran its designed sequence — safety dump → stop → start → replay — and the -replay aborted: - -``` -10:58:25 controller: replaying DB dump into immich-postgres -10:58:33 immich-server: "Reindexing clip_index" -> "Reindexed clip_index" -10:58:35 controller: ERROR relation "clip_index" already exists - exit status 3 -``` - -`ImportDump` needs a running database container, so the code started the WHOLE stack first. That gave -the application an eight-second window to rebuild the very schema objects the dump was about to -create; under `ON_ERROR_STOP=1` the collision aborted the script. The data survived only because -`pg_dump` emits COPY before CREATE INDEX — a collision earlier in the script would have left a -genuinely half-restored database and reported it identically. - -**Class defect.** The local `RestoreFromRecoveryUnit` had the same start-then-replay shape, hidden -inside `RecreateStackFromUnit` (which ended in a full `compose up -d`). Both are fixed here. - -## 2. What was built - -**Part 1 — the seams** - -| Change | File | +| Leg | Status | |---|---| -| `dbTypeForImage` extracted from `DiscoverDatabases` (behaviour byte-equivalent) and shared | `internal/appbackup/dbdump.go`, `internal/appbackup/dbservices.go` (new) | -| `DBServiceNames(composePath)` — sorted compose SERVICE names holding a DB; yaml.v3 `services:` map parse | `internal/appbackup/dbservices.go` (new) | -| `Manager.StartStackServices(name, services)` — scoped `up -d`, **refuses an empty list** | `internal/stacks/manager.go` | -| `RedeployFromEnv` split; persist half is `PersistUnitRedeployConfig` (starts nothing) | `internal/stacks/deploy.go` | -| `StackDataProvider`: `RecreateStackFromUnit` → `RecreateStackDefinitionFromUnit` (+ `StartStackServices`) | `internal/appbackup/appdata.go` | -| Adapter: definition-only recreate + delegation | `cmd/controller/main.go` | -| `DBServiceNames` forwarder | `internal/backup/appbackup_bridge.go` | +| **Part 1 — R-48 restore wizard (controller v0.154.0)** | **DONE.** Committed `3a9d744`, pushed, image `0.154.0` built + pushed. **Deliberately NOT deployed** (by design — the floor save is the deploy). | +| **Part 2 — capability-map cell fix (felhom.eu)** | **DONE.** Committed `ce8c539`, pushed. | +| **Part 3 — agent 0.90.1 publish + deploy** | **STOPPED before publishing — premise does not hold.** See §5. Nothing published, nothing deployed, felhom-pve untouched. | +| **STOP-1 — floor save + single-fire assertion** | **PENDING — needs the operator.** Preconditions verified green (§2). | +| **Phase D — the two post-evidence doc flips** | **NOT DONE**, correctly: both are gated on evidence that does not exist yet (§7). | -**Part 2 — offsite** (`internal/backup/offbox_reconstitute.go`): DB services resolved from the LIVE -compose before any mutation; fail-closed refusal when a DB exists but no service is identifiable; -sequence is now **stop → files → `StartStackServices(dbServices)` → replay → `StartStack` (full) → -health wait**; both failure exits from the window do a best-effort full start. +## 1. Baselines (re-confirmed live at session start) -**Part 3 — local** (`internal/backup/restore_unit.go`): DB services resolved from the UNIT's compose -(it is about to become the live one) plus `hasReplayableDump` (excludes `pre-restore-` safety dumps); -same fail-closed gate before the first mutation; sequence is now **stop → volumes → -`RecreateStackDefinitionFromUnit` → `StartStackServices` → replay → `StartStack` (full) → health -wait**, with the pre-existing `dataErr` / "completed with data errors" semantics preserved. +| Repo | `main` @ start | Clean + synced | End state | +|---|---|---|---| +| felhom-controller | `b30e2e5` | yes | `3a9d744` (v0.154.0) | +| felhom.eu | `fb0b8c1` | yes | `ce8c539` | +| felhom-agent | `8c55ac7` | yes | **unchanged — no commit, no publish, no deploy** | -Untouched, as specified: `restore_db.go`, `ImportDump`, `waitDBReady`, the dump flags -(`--clean --if-exists`, `ON_ERROR_STOP=1`), `mapOffsiteRestorePaths`, the copiers, the honesty -surfaces, `IsDownState`/alerting (R-51), and the agent/hub. +## 2. Phase-0 probes -## 3. Tests — 19 new, Groups A–G +### P1 — self-update enabled and hub-wired on guest 9201: **PASS** + +From `/var/lib/docker/volumes/felhom-controller-data/_data/controller.yaml`: + +```yaml +self_update: + auto_update: false + check_interval: 6h + enabled: true + health_timeout_seconds: 60 + image: gitea.dooplex.hu/admin/felhom-controller +``` + +`auto_update: false` is **not** a blocker, and this was verified by reading the code rather than +assumed: `MaybeAutoUpdate` (`internal/selfupdate/updater.go`) never consults `cfg.AutoUpdate`. That +flag is the customer's opt-in to chase *latest*; the FLOOR path is the managed one and is independent +of it. + +Stronger evidence than the config — the path has already fired on this box. +`data/update-state.json`: + +```json +{ "status": "success", "previous_version": "0.143.0", "target_version": "0.145.0", + "initiated_by": "auto-floor", "initiated_at": "2026-07-18T16:32:29Z", + "completed_at": "2026-07-18T16:32:34Z" } +``` + +`initiated_by: "auto-floor"` proves the agent swapper is wired (a nil agent short-circuits with +"no agent — auto-update unavailable") and that a floor-driven swap completes in ~5 s on this box. + +Anti-flap will **not** block the 0.154.0 swap: the persisted `target_version` is `0.145.0`, not +`0.154.0`, and the in-process `lastAutoFloorAttempt` is unset for it (the current-≥-floor branch +returns before setting it). Live guest state at probe time: image `0.153.0`, `Up 15 hours (healthy)`, +container `StartedAt 2026-07-20 15:03:11 UTC`. + +Registry precondition for the floor validation also verified: `0.154.0` is the **highest** semver tag +in `admin/felhom-controller` (23 tags; top four `0.151.0, 0.152.0, 0.153.0, 0.154.0`), so the +`floor <= latest` gate passes rather than deferring. + +### P2 — agent version: **PASS, with a finding that changed the plan** + +`cmd/felhom-agent/main.go:59` holds `var version = "0.89.0"` as a *fallback only*; the real version is +ldflags-injected (`-X main.version`) by `scripts/publish-agent.sh`. Built at `main` and verified by +self-report, not by CHANGELOG: + +``` +$ felhom-agent --version +felhom-agent 0.90.1 +sha256 ba1d029602c96b1b743a4dca3ddbd9b63415fcb47021b91a21f8c32dcc534870 13 734 067 bytes +``` + +The R-39 fix commit `9596d5a` is an ancestor of `main` and is not reverted. Agent green gate passes. +**The finding is in §5** — this binary does not contain the R-39 fix, and cannot. + +### P3 — before-state of `/backups/restore`: **CAPTURED** + +Method: **endpoint-level** (no browser on DooPlex). Authenticated session against the container IP +with the `Host:` header, from inside guest 9201. `LOGIN_HTTP=302`, `PAGE_HTTP=200`, 58 115 bytes. +Offsite section, buttons in render order — this **is** the R-48 defect: + +``` +immich / bookstack / calibre-web each: + - Visszaállítás ellenőrzéshez (konfiguráció + adatbázis) + - Teljes visszaállítás előkészítése +immich additionally (scratch prepared): + - Helyreállítás az élő adatok közé (csak a hiányzó fájlok) <- data CANNOT come back + - Teljes visszaállítás (fájlok + adatbázis) <- data CAN come back +``` + +Form actions on the page: 6× `/backup/offbox/restore`, 1× `place`, 1× `reconstitute`. The two decisive +controls are adjacent siblings differing only by label. Snapshot retained at +`scratchpad/restore-before.html`. + +## 3. Part 1 — what shipped (commit `3a9d744`) + +Files touched: `internal/web/restore_wizard.go` (new), `templates/backups_restore_wizard.html` (new), +`internal/web/restore_wizard_test.go` (new), `templates/backups_restore.html`, `offbox_handlers.go`, +`server.go`, `templates/style.css`, plus `CHANGELOG.md` / `CONTEXT.md` / `REUSE.md` / +`controller/README.md`. + +- **One entry per app.** The five inline forms per row collapse to a single „Visszaállítás…" link to + `GET /backups/restore/app?name=`. +- **Three intent CARDS** with consequence sentences, danger styling on card 3, the R-43 + double-confirm and its pair-honesty facts carried over **verbatim**. +- **`deriveWizardStep` is pure** over (op running, size-gate flash, scratch ready). Precedence is + strict and load-bearing: a running op outranks a stale `?full_prep=`. +- **No new mutation endpoint.** One GET route added; every card posts to the pre-existing + `/backup/offbox/{restore,place,reconstitute}` with unchanged field names and gates. +- Untouched as instructed: the shares block, the local restore panel, the .fab block, + `internal/backup`, `internal/appbackup`, `internal/selfupdate`. + +**Bug found and fixed on the way (not in the spec).** `offboxRedirectTo` hardcoded `"?"` when +appending its flash. Retargeting redirects at a URL that already carries `?name=` would have +produced `...?name=immich?flash=...`, burying the flash inside the `name` value — the wizard would +then have refused its own app with "nincs kijelölve" after every action. The separator is now chosen. + +### Test results + +Full suite green: `go build ./... && go vet ./... && go test ./...` — **0 failures**. +All six controller design gates pass (`template_id`, `emoji`, `mojibake`, `native_confirm`, +`offbox_rename`, `app_row_dedup`). | Group | Test | Result | |---|---|---| -| A | `TestReconstituteReplaysWithOnlyTheDBServiceUp` — order **plus state-at-replay-time** | PASS | -| A | `TestReconstituteReplaysDBAndOrdersOperations` (existing, sequence assertion updated) | PASS | -| B | `TestReconstituteNoDBAppNeverStartsServicesOnly` — negative, zero scoped starts | PASS | -| C | `TestReconstituteRefusesWhenNoDBServiceIdentifiable` — zero-mutation effect | PASS | -| C | `TestRestoreFromUnitRefusesWhenNoDBServiceIdentifiable` — zero-mutation effect | PASS | -| D | `TestRestoreFromUnitReplaysWithOnlyTheDBServiceUp` | PASS | -| D | `TestRestoreFromUnitNoDumpsTakesOneFullStart` | PASS | -| D | `TestRestoreFromUnitIgnoresSafetyDumpsWhenDecidingToReplay` | PASS | -| E | `TestReconstituteReplayFailureStillBringsTheStackUp` | PASS | -| E | `TestReconstituteDBOnlyStartFailureStillBringsTheStackUp` | PASS | -| E | `TestRestoreFromUnitReplayFailureStillBringsTheStackUp` | PASS | -| F | `TestDBTypeForImage`, `TestDBServiceNames` (8 sub-cases), `TestDBServiceNames_TopLevelKeysAreNotServices`, `TestDBServiceNames_UnreadableAndUnparseableError`, `TestDiscoverAndComposeAgreeOnTheSameImages` | PASS | -| G | `TestStartStackServicesRefusesEmptyList`, `TestPersistUnitRedeployConfigPersistsWithoutStarting`, `TestPersistUnitRedeployConfigRejectsUnknownStack` | PASS | +| B | `TestDeriveWizardStep_Table` (7 rows) | PASS | +| C | `TestResolveWizardApp_Refusals`, `TestRestoreWizardHandler_UnconfiguredRedirects` | PASS (302, no 500) | +| A | `TestRestoreList_SingleEntryPerApp` | PASS | +| C | `TestRestoreWizard_ThreeIntentCards`, `TestRestoreWizard_NoScratchLocksDataIntents` | PASS | +| E | `TestRestoreWizard_OpRunningSuppressesAllMutations` | PASS | +| D | `TestRestoreWizard_NoNewMutationEndpoints`, `TestRestoreWizard_FieldContract` | PASS | -The core assertion is deliberately not "no error": a recording provider captures whether the FULL -stack had been started at the moment the import fired. Asserting only `err == nil` passes on the -pre-fix shape — which is exactly how this shipped. +Two pre-existing tests were coupled to the old IA and were updated, not deleted: +`TestAppRow_RestoreLists` and `TestBackupsSplit_SectionsOnExactlyOnePage` asserted +`action="/backup/offbox/restore"` on the list page — now inverted to assert those forms are **absent** +there and the single wizard entry is present. -The compose-parser decoys use the catalog's REAL immich template shape (`immich_ml_cache:`, -`immich_postgres_data:` as top-level `volumes:` keys, `ghcr.io/immich-app/postgres:16-vectorchord…` -as the pin) — the exact input a line scan would misread. +### Group-B red-proof (run, then reverted) -### Companion red-proofs — three run, all reverted, tree clean - -| # | Pre-fix shape restored | Failure observed | -|---|---|---| -| 1 | offsite: `StartStackServices` → full `StartStack` before the replay | `TestReconstituteReplaysWithOnlyTheDBServiceUp`: *"the database service was NOT started before the replay"*; `TestReconstituteReplaysDBAndOrdersOperations`: sequence `"stop,start,start"` | -| 2 | local: full `StartStack` inserted before the replay | `TestRestoreFromUnitReplaysWithOnlyTheDBServiceUp`: *"the FULL stack was already up when the replay fired — the H4 race, on the local path"* | -| 3 | both fail-closed gates deleted | both `RefusesWhenNoDBServiceIdentifiable` tests: *"expected a refusal…"* | - -### Green gate - -`go build ./... && go vet ./... && go test ./...` — **23/23 packages green**, exit 0 -(`internal/backup` 174 s). New tests by package: backup +11, appbackup +5, stacks +3. - -## 4. Deployment - -Built + pushed from the clean tree at `78ff991`: `gitea.dooplex.hu/admin/felhom-controller:0.153.0`, -digest `sha256:cc02c950c47456dac01aa754f23befbdfb3d4897b0a9863aafa3589cff95b8ab`. Deployed to guest -9201 via the bootstrap path (pull → `/etc/felhom-controller-image` → restart bootstrap service). -Verified: `gitea.dooplex.hu/admin/felhom-controller:0.153.0 | Up (healthy)`, clean startup, and -`Event pushed: controller_started (info) — Controller elindult (0.153.0)`. - -## 4b. STOP-1 — supervised live leg: **PASSED** (2026-07-20, operator present) - -Method: **endpoint-level** — the exact endpoints the UI posts to, driven with an authenticated -session inside the guest (no browser on DooPlex; no state hand-setting, no `docker exec` shortcut -around the pipeline). App: **immich** (a real DB-indexed app), snapshot `49e7cb46` — the SAME -snapshot that aborted in round 2. - -Baseline before the run: **11 assets, all `active`**. - -1. `POST /backup/offbox/restore` `app=immich mode=full confirm=1` → 302; scratch staged - **15:30:08 UTC**: `restored immich (49e7cb46, full=true) → …/backups/offsite-restore/immich`. -2. `POST /backup/offbox/reconstitute` `app=immich confirm=1` → 302, fired **15:39:36 UTC**. - -**Controller log — the ordering, live:** +Replaced the body of `deriveWizardStep` with the trivial `return restoreWizardView{Step: +wizStepIntent, VerifyEnabled: true}`. Result — **all 7 table rows FAILED**, plus the execution render +test: ``` -15:39:42 [offbox] immich: pre-restore safety dump written -> pre-restore-20260720T153941Z-immich-postgres.sql (49.9 MB) -15:39:42 [stacks] Stopping stack: immich -15:39:43 [stacks] Starting stack immich services only: [immich-postgres] <- THE FIX -15:39:43 [backup] Restore immich: replaying DB dump into immich-postgres (postgres) -15:40:03 [backup] Restore immich: replayed 1 DB dump(s) <- rc-0, 20 s -15:40:03 [stacks] Starting stack: immich <- full start, only now -15:40:27 [offbox] reconstituted immich from snapshot 49e7cb46: 6 file(s) placed, - 1 DB dump(s) replayed, safety dump=pre-restore-…, skewed=false +--- FAIL: TestDeriveWizardStep_Table/op_running_(this_app)_→_execution;_nothing_offered +--- FAIL: TestDeriveWizardStep_Table/op_running_OUTRANKS_a_stale_full_prep_flash… +--- FAIL: TestDeriveWizardStep_Table/scratch_ready_→_intent,_and_BOTH_data-touching_intents_unlock… +--- FAIL: TestDeriveWizardStep_Table/full_prep_flash_for_THIS_app_→_prepare-confirm… + (+3 more rows) +--- FAIL: TestRestoreWizard_OpRunningSuppressesAllMutations + restore_wizard_test.go:253: the execution step must render NO mutation form + restore_wizard_test.go:256: the execution card must name what is actually running ``` -**Verification through the system's own surfaces:** +Implementation restored; suite green; `git diff` clean before commit. -| Check | Round 2 (v0.148.0) | This run (v0.153.0) | -|---|---|---| -| `already exists` / replay abort | `ERROR relation "clip_index" already exists` (exit 3) | **none** — replay exited 0 | -| App up during replay | yes (immich-server rebuilt `clip_index` mid-replay) | **no** — only `immich-postgres` was up | -| Operation outcome | reported FAILURE | **success**, `dbs=1` | -| immich's own schema verdict | **schema drift** reported | **`No schema drift detected`** — twice (Microservices + Api) | -| Assets | 11 (recovered by luck of `pg_dump` ordering) | **11, all `active`** | -| Containers | — | all four immich containers `Up (healthy)` | -| `public` indexes | incomplete (aborted script) | **231** | +One assertion in `TestRestoreWizard_ThreeIntentCards` was itself caught being hollow during +authoring: a bare substring check for the skew/empty warnings passed on a *clean* pair, because the +`confirmFullRestore` JS repeats both sentences as string literals. Tightened to assert the rendered +banner markup and the `data-restore-*` attributes instead. -The decisive line is `Starting stack immich services only: [immich-postgres]` followed by a replay -that exits 0 — the window in which H4 occurred no longer exists. The DB service name was resolved -from the LIVE compose by `DBServiceNames`, unassisted. +### Image -Credentials handling per GL-1: the operator's password was supplied file→file, never echoed, and the -file plus both cookie jars (host and guest) were shredded at the end of the run. - -## 4c. Phase C — golden **0.153.0** baked and published (2026-07-20) - -Probes first, all before any mutation: - -| Probe | Result | -|---|---| -| **P1** — read `180:/mnt/5_hdd/felhom.eu/drill/bake-0.146.0.log` end to end | recipe extracted: `pct`-based LXC 9100 on felhom-pve, args `9100