Files
felhom-controller/REPORT.md
T

255 lines
15 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# REPORT — R-48 restore wizard (v0.154.0) + the v0.155.0 follow-up fix; STOP-1 BANKED; Part 3 STOPPED
Session date: **2026-07-21**. Executed on DooPlex as `kisfenyo`.
## 0. Outcome at a glance
| Leg | Status |
|---|---|
| **Part 1 — R-48 restore wizard (v0.154.0)** | **SHIPPED** `3a9d744`, image published, deployed via the floor save. |
| **v0.155.0 — follow-up fix** | **SHIPPED + DEPLOYED** `9d1b498`. Fixes a real defect in v0.154.0 found by the operator on the first live click-through. |
| **Part 2 — capability-map cell** | **DONE** `ce8c539`. |
| **STOP-1 — floor save + single-fire** | **BANKED.** Exactly one swap over a 39-min window, no rollback, healthy. §6. Doc rows flipped `11ead4b`. |
| **Live click-through + Ellenőrzés** | **DONE by the operator**, and re-run endpoint-level after the v0.155.0 fix. §7. |
| **Part 3 — agent 0.90.1** | **STOPPED, ruled.** Not published, not deployed. §8. |
## 1. Baselines
| Repo | start | end |
|---|---|---|
| felhom-controller | `b30e2e5` | `9d1b498` (v0.155.0) |
| felhom.eu | `fb0b8c1` | `c35da9d` |
| felhom-agent | `8c55ac7` | **unchanged — no commit, no publish, no deploy** |
## 2. Phase-0 probes
**P1 — self-update wiring: PASS.** `self_update.enabled: true`, image correct. `auto_update: false` is
irrelevant and this was verified by reading the code, not assumed: `MaybeAutoUpdate` never consults
`cfg.AutoUpdate` — that flag is the customer's opt-in to chase *latest*, while the FLOOR path is the
managed one. Stronger evidence: `update-state.json` already recorded a prior `initiated_by: auto-floor`
success (0.143.0 → 0.145.0, 2026-07-18), which proves the agent swapper is wired. Anti-flap could not
block, since the persisted target was `0.145.0`. `0.154.0` verified as the highest registry semver tag,
so the `floor <= latest` gate would pass.
**P2 — agent version: PASS, and it changed the plan.** Built at `main`, self-reported
`felhom-agent 0.90.1`, sha256 `ba1d0296…`, 13 734 067 bytes. But see §8.
**P3 — before-state: CAPTURED** (endpoint-level; no browser on DooPlex). The offsite section rendered
**6× `/backup/offbox/restore`, 1× `place`, 1× `reconstitute`** across three apps. For immich the two
decisive controls were adjacent siblings differing only by label:
```
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
```
## 3. v0.154.0 — the wizard (`3a9d744`)
One „Visszaállítás…" entry per app row → `GET /backups/restore/app?name=<app>`; three intent cards
with consequence sentences; danger styling and the R-43 double-confirm carried over verbatim; pure
`deriveWizardStep`; **no new mutation endpoint**; works without JavaScript. `internal/backup`,
`internal/appbackup`, `internal/selfupdate`, the shares/local/.fab blocks all untouched.
**Latent bug fixed on the way:** `offboxRedirectTo` hardcoded `"?"` when appending its flash, which
against the wizard's `?name=<app>` target would have buried the flash inside the app name.
**Group-B red-proof (run, reverted):** trivial always-INTENT impl → **all 7 table rows FAILED** plus
the execution render test. Restored, green, `git diff` clean before commit.
Also caught one of my own assertions 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 rendered banner markup + `data-restore-*`.
## 4. v0.155.0 — the defect v0.154.0 shipped (`9d1b498`)
**Found by the operator on the first live click-through**, from the phase strip disagreeing with the
progress banner on the same screen. This is the most important finding of the session.
`backup.Manager` carries **two** different "is something running" booleans:
| flag | read by | set by | covers the verification restore? |
|---|---|---|---|
| `running` | `IsRunning()` | `acquireRunning()`, **inside** the goroutine | **no — `RestoreOffboxScratch` never acquires it at all** |
| `opRunning` | `RestoreStatus()` | `BeginRestoreOp()`, in the handler, synchronously | yes, all four offsite actions |
v0.154.0 sourced `OpRunning` from `IsRunning()`. Verified by reading the call sites:
`PlaceOffsiteRestore` and `ReconstituteFromOffsite` do acquire; **`RestoreOffboxScratch` does not**.
So for „Ellenőrzés" and the full-restore preparation — the wizard's two most-used and longest actions,
both streaming from restic — the execution step was **unreachable**. The page offered all three
intents with live buttons while a restore ran; the handler would have refused any of them. That is
precisely the "offering a control guaranteed to fail" dishonesty R-48 exists to remove, shipped inside
the release that removes it.
**Fix:** `restoreOpInFlight(st)` behind a documented seam, fed by a **single** `RestoreStatus()` read
per render, so the strip, the suppression decision and the running-op name cannot diverge. The helper
sits on the handler's actual path — deliberately, so a future edit reintroducing `IsRunning()` fails
the test rather than passing it.
**Why v0.154.0's tests missed it — the generalisable lesson.** The Scenario-E table proved
`deriveWizardStep` behaves correctly *given* `OpRunning=true`. Nothing proved the handler ever
*computes* `true`. **A table test over a pure function proves the function, not the caller.** The new
`TestRestoreOpInFlight_UsesDisplayFlagNotConcurrencyFlag` drives a real `Manager` through
`BeginRestoreOp` and asserts the render.
**Red-proof (run, reverted):** restoring the v0.154.0 `IsRunning()` shape →
```
--- FAIL: TestRestoreOpInFlight_UsesDisplayFlagNotConcurrencyFlag
restore_wizard_test.go:367: a started restore op MUST read as in-flight for display
(this is the v0.154.0 bug)
```
**„Eredmény" was a dead label.** It never lit in v0.154.0. The strip's highlight is now its own
derived `Phase`, separate from `Step`: a finished restore is back on the intent step (everything
available again) while the strip reads „Eredmény" and an outcome card shows the result. Bounded by
`restoreResultWindow` (10 min) so a stale result cannot look fresh, and **bound to the app** so a
finished bookstack restore cannot light immich's page with bookstack's message. The card survives a
reload; the flash does not. 9-row table test incl. both boundary directions and a future-clock-skew row.
## 5. Test + gate results
Full suite green at both commits: `go build ./... && go vet ./... && go test ./...` — 0 failures.
All six controller design gates rc=0.
| Group | Test | Result |
|---|---|---|
| B | `TestDeriveWizardStep_Table` (9 rows) | PASS |
| C | `TestResolveWizardApp_Refusals`, `TestRestoreWizardHandler_UnconfiguredRedirects` | PASS (302, no 500) |
| A | `TestRestoreList_SingleEntryPerApp` | PASS |
| C | `TestRestoreWizard_ThreeIntentCards`, `..._NoScratchLocksDataIntents` | PASS |
| E | `TestRestoreWizard_OpRunningSuppressesAllMutations` | PASS |
| D | `TestRestoreWizard_NoNewMutationEndpoints`, `..._FieldContract` | PASS |
| **new** | `TestRestoreOpInFlight_UsesDisplayFlagNotConcurrencyFlag` | PASS (red-proofed) |
| **new** | `TestHasRecentRestoreResult` (9 rows), `TestRestoreWizard_ResultCard` | PASS |
Two pre-existing tests coupled to the old IA were **inverted, not deleted**: `TestAppRow_RestoreLists`
and `TestBackupsSplit_SectionsOnExactlyOnePage` now assert the mutation forms are ABSENT from the list.
## 6. STOP-1 — single-fire self-update: **BANKED**
Operator saved global floor → v0.154.0. Timeline (host CEST / guest UTC, 2 h offset):
```
06:57:13Z UpdateState "pending" initiated_by=auto-floor
06:57:17Z agent WARN controller swap requested 0.153.0 -> 0.154.0
06:57:19Z agent INFO image file written, restarting bootstrap
06:57:21Z container StartedAt · UpdateState completed_at
06:57:29Z agent INFO new controller healthy
```
| Assertion | Result |
|---|---|
| `controller swap requested` | **1** |
| `image file written, restarting bootstrap` | **1** |
| `new controller healthy` | **1** |
| rollback / swap-failed / unhealthy | **0** |
| container `RestartCount` | **0** |
| bootstrap unit `Result` | `success` |
> **Correction — I over-claimed one line.** I originally cited the bootstrap unit's `NRestarts=0` as
> no-storm evidence. It isn't: the unit is `Restart=no`, so `NRestarts` is 0 by construction and would
> read 0 during a storm too. Dropped. The real no-storm evidence is the agent-side swap count and the
> container's own `RestartCount=0`.
`UpdateState` persisted `status: success`, `0.153.0 → 0.154.0`, `initiated_by: auto-floor`.
`VerifyStartup` banked it on the next boot (`Post-update startup: update successful`), and the 06:57:52
periodic check reported `Current version 0.154.0 is up to date` — the anti-flap correctly doing nothing
now that current ≥ floor. **Exactly one swap, no storm, no rollback.** This closes the leg R-23(a) was
missing: the earlier floor save proved nothing because it targeted an already-running version.
**30-minute no-re-attempt check — PASSED, and my earlier caveat was broader than the facts.** Recounted
over 06:50 → 07:29 (39 min): `controller swap requested` = **1**, agent-driven bootstrap restarts = **1**.
I hand-deployed v0.155.0 at `07:17:10Z`, inside the observation window, and flagged that as
contaminating the result. Having checked the scope: it contaminates **`StartedAt` only**. A hand-deploy
writes `/etc/felhom-controller-image` and restarts the unit directly via `pct exec` — it never goes
through `SwapController` — so the swap-count assertions, which are the actual evidence, hold
uncontaminated across the whole window. I was right to flag it and wrong about how much it cost; the
window is clean, not merely the 19 minutes I claimed.
**An unplanned bonus confirmation came with it.** After the hand-deploy the box ran **0.155.0 against a
0.154.0 floor** — i.e. above the floor — and the updater logged `Current version 0.155.0 is up to date`
and did nothing. That is Scenario B ("at/above floor does NOTHING — must not chase latest") confirmed
live, which no test in this session set out to produce.
## 7. Live validation
Method: **endpoint-level** (no browser on DooPlex) plus the operator's own UI click-through.
**Operator, in the browser (v0.154.0):** list page → one „Visszaállítás…" per app; wizard → three
cards, pair-honesty panel showing `adatbázis-mentés ideje: 2026-07-19 12:28`; ran one **Ellenőrzés**;
result `A(z) immich visszaállítva ellenőrző mappába: /mnt/felhom-drives/hdd_1/backups/offsite-restore/immich
(a meglévő adatok változatlanok).` Non-destructive as designed.
**Endpoint-level, after the v0.155.0 fix — the proof the defect is gone.** A real Ellenőrzés run
through the actual endpoint:
| moment | phase strip | `<form>` count | note |
|---|---|---|---|
| before | Előkészítés | 3 | intents offered |
| **mid-flight** | **Végrehajtás** | **0** | `Jelenleg egy mentési vagy visszaállítási művelet fut (immich).` |
| after | **Eredmény** | 3 | result card names the scratch path; intents available again |
| bookstack, same moment | Előkészítés | — | **no** result card — app-binding holds |
Under v0.154.0 the mid-flight row would have read `Előkészítés / 3 forms`. Before/after on the list
page: **6 restore + 1 place + 1 reconstitute forms → 0**, and 3 wizard entries. Refusal path: unknown
app → **302** + Hungarian flash, no 500.
Deploy verified: `felhom-controller:0.155.0 | Up (healthy)`, `RestartCount=0`.
## 8. Part 3 — STOPPED, then ruled
Part 3 asked me to publish + deploy v0.90.1 as "R-39's cheap half — the PBS wrapper argv fix", while
Rule 12 forbade touching the wrapper. Both cannot hold, because **the fix IS the wrapper**:
- **`9596d5a` changes zero non-test Go files** — `CHANGELOG.md`, `REPORT.md`, `configs/felhom-pbs-apply`,
`internal/pbsdr/manager_test.go`. Its own message: *"the Go binary is unchanged."*
- **felhom-pve already has it** — wrapper hotfixed 2026-07-18, `args=(--fingerprint "$fp")` at L107,
`.bak-20260718-preR39` retained.
- **New hosts get it too, unversioned** — `felhom-host-install.sh:1914` `fetch_raw` pulls the wrapper
from `raw/branch/main`, and `9596d5a` is an ancestor of `main`.
Publishing would have delivered no behaviour change, restarted a production agent at a remote site for
nothing, and — once the Day-0 manifest was saved to 0.90.1 — advertised a versioned fix the artifact
channel never carried. Registry pre-check confirmed and left as found (0.90.1 → **404**, 0.90.0 → 200).
**Operator ruling: leave 0.90.0 published; record the closure in the docs.** Done in `ROADMAP.md`
(`c35da9d`). The stop surfaced a genuine new defect, filed as **R-50b**: a root-owned privileged host
artifact is delivered unversioned from `main`, absent from the Day-0 manifest, so "which wrapper is on
this host?" is unanswerable from any manifest.
## 9. Doc state
- Capability map row 61 → **PROVEN-LIVE** (`ce8c539`).
- ROADMAP: **R-48 → SHIPPED**; R-39 cheap half recorded as closed-in-field with the publish cancelled;
**R-50b** filed (`c35da9d`).
- **FLIPPED** (`11ead4b`): R-23(a) → **BANKED in full** (only the cosmetic (b) leg remains), and the
capability-map row "Config/state change round-trips in **seconds**" → **PROVEN-LIVE (2026-07-21)**.
That row's residual PARTIAL was explicitly "the R-23 operator-UI save→apply leg alone", which §6
closes. Both rows carry the hand-deploy disclosure and the bonus at/above-floor confirmation.
## 10. Remaining — human
1. **Floor → v0.155.0** at your convenience, so the fleet matches the box (9201 is hand-deployed and
above floor, so nothing is pending on it).
2. **TASK-B** ruling (R-39 primary half: the descriptor-hash re-apply gap).
3. **R-50b** ruling — recommendation is (a): record the wrapper's sha256 in the Day-0 manifest and have
the agent report the installed file's hash, so drift becomes visible.
4. **C6 customer-restore pass** for the alpha.
## 11. Observations
- **The most valuable finding came from the operator, not the tests.** The phase strip he questioned
was the visible symptom of a real bug — the strip and the progress banner disagreeing on one screen.
A UI element that renders derived state is an assertion the customer can check; that is worth more
than its decorative cost.
- **`docker_run_volume_path_gate.py` fails at `HEAD`, unrelated** (`internal/appexport/estimate.go:179`).
Verified pre-existing by stashing and re-running. Left alone — but that gate is currently not a
usable signal.
- **`opstatus.go` says in its own header that it is the DISPLAY surface** and does not gate
concurrency. I read the concurrency flag anyway. The file told me; I did not check.
- **`felhom-pve` is up and healthy** (agent 0.90.0, `active`) — the "agent is DOWN" TEMPORARY block in
`felhom-controller/CLAUDE.md` is stale and should be trimmed on return.
- **P3's accented-grep trap fired as documented** — used a local Python pass over fetched bytes rather
than grepping accented patterns through `ssh → pct exec → bash -c`.