From 4aa2ce4b61235edecf2c0e043773dcd5c777543a Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Tue, 21 Jul 2026 16:36:14 +0200 Subject: [PATCH] docs(report): lifecycle implementation, the v0.158.1 defect, and the R-55 live leg --- REPORT.md | 203 +++++++++++++++++++++++++++++------------------------- 1 file changed, 108 insertions(+), 95 deletions(-) diff --git a/REPORT.md b/REPORT.md index 12d66a1..5cc699c 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,122 +1,135 @@ -# REPORT — R-55 boot-gate stop-honesty (TASK-E Part 2), 2026-07-21 +# REPORT — app lifecycle states + R-55 live leg (TASK-F Parts 1 & 3), 2026-07-21 -**Versions: v0.157.0 (R-55) + v0.157.1 (.gitignore anchor). Baseline `83f20c8` (v0.156.0), clean tree. -v0.157.0 is BUILT, PUSHED and DEPLOYED to guest 9201.** +**Versions: v0.158.0 (lifecycle) + v0.158.1 (a defect I shipped in 0.158.0 and caught live — read §4). +Baseline `a96226a` (v0.157.0), clean tree. Deployed to guest 9201 and live-verified.** -## Scope correction — this part was specified against the wrong repo +## 1. What shipped — app lifecycle -TASK-E placed R-55 in `felhom-agent` (Parts 1–2 → felhom-agent) and separately forbade controller -changes ("felhom-controller untouched", "no controller changes at all"), with a supervised-lite leg -described as an *agent binary* deploy to felhom-pve. +`.felhom.yml` gains an optional top-level `lifecycle:`. Absent/empty ≡ `available`, so all existing +templates are untouched. -**R-55 is not in the agent.** It is `felhom-controller/controller/internal/web/intermediary.go` — -the in-guest boot bind gate. The agent's only related code is host-side `ReassertGuestBinds`, which -re-adds `pct set -mpN` binds and never touches apps. The GO ruling and the scope fence therefore -contradicted each other; the fence was evidently written believing the gate lived in the agent. +| state | offered for new installs | shown to someone already running it | +|---|---|---| +| `available` | yes | nothing | +| `hidden` | **no** | nothing — "we stopped offering this" is not their problem | +| `abandoned` | **no** | „Nem karbantartott" badge + a permanent notice on the app page | -**Raised as a STOP rather than resolved unilaterally**, because it changes which repo ships and which -artifact deploys. **Operator ruling 2026-07-21: implement in the controller AND deploy to 9201.** Done. +**A deployed instance keeps full function in every state.** That is the whole design: the earlier +`retired/` directory move un-offered the app but would ALSO have made the orphan detector see the +template as gone for anyone running it — flagging a working install `Elavult` and offering a Törlés +button. Withdrawing an app must never take a working app away from a customer. -## The defect +Three design points worth carrying past this feature: -`shouldRecreateOnBoot` keyed on `Deployed && HDD_PATH && drive-present`. `Deployed` is a -deploy-lifecycle flag that stays true across a Stop, so the gate could not tell "the guest went down -under this app" from "the customer switched this off" — and resurrected both. Proven live by STOP-1's -R-52 leg, which was designed to prove the opposite: immich, stopped from the UI seconds earlier, came -back running after `pct reboot 9201`. +- **The deploy gate is server-side and fail-CLOSED**, before any mutation, with the ruled message + „Ez az alkalmazás jelenleg nem telepíthető." Hiding a button is not a gate — a stale link, a + bookmarked deploy form and a direct POST must all be refused. A second check in + `stacks.DeployStack` covers callers that do not route through the API. +- **Unknown values fail OPEN** (→ `available` + one WARN) — deliberately the opposite posture. A + typo, or a state from a catalog newer than this controller, must never silently pull a working app + out of every customer's list. Both paths read the same `EffectiveLifecycle`, so they cannot disagree. +- **Lifecycle never reaches orphan detection.** `getCatalogTemplateSlugs` keys on directory + compose + presence only. Red-proofed. -The operator's rationale for ruling this GO is worth keeping attached to the fix: an unconditional -recreate can **drown the node** when apps were deliberately stopped to free resources for others. This -is not only about honesty; it is about not fighting the customer's own capacity management. +Badge plumbing is generic (`MetaBadge` + the `meta_badge` partial + a `lifecycleBadge` funcmap +entry) so R-56's difficulty labels need only a sibling funcmap function — no new markup or CSS. -## Spike-lite: a truthful source already existed — nothing was invented +## 2. Live validation — endpoint-level, on 9201 -The task required stopping rather than inventing state if no honest running-at-shutdown signal existed. -It does exist, and R-52 already trusts it: - -- `AppConfig` carries **no** run-state. `Deployed` is lifecycle, not intent. -- `Stack.Containers` comes from `docker ps -a` (so `Exited` containers **count**), and docker's own - on-disk container records survive a reboot. -- A UI Stop is `compose down`, which **removes** the containers. Nothing else leaves a deployed app at - zero containers. - -That is exactly `bootrecon.isBootOrphan`'s `existing-Exited vs absent` distinction. The gate now -additionally requires `len(Stack.Containers) > 0`. - -**What deliberately did NOT change: container STATE is still not a filter.** That half of the original -design is load-bearing — a `State != stopped` filter misses an app docker has not auto-restarted yet, -or one stuck `Exited` on a create-time bind failure with `RestartCount=0`, which is the bug the boot-id -path exists to fix. `state` and `hasContainers` are different questions ("is it up right now" vs "does -docker still have records of it") and only the second survives a reboot as a statement of intent. -`TestShouldRecreateOnBoot` now pins both axes at once; they pull in opposite directions, which is the -entire difficulty of this gate. - -**Ordering trap, found and handled:** the evidence is sampled into the `bootStack` snapshot **before** -any recreate runs, because `recreate` itself calls `StopStack` (`compose down`) and destroys it. - -**No regression to the drive-absent gate:** apps it stopped are also at zero containers, so this path -now skips them — correctly. They are recorded in `StoragePath.StoppedStacks` and restarted by -`ReconcileDriveGates`' `Return` branch on the same `driveGateLoop` tick. Checked, not assumed. - -**Honoured Stops are observable.** `leftStopped` is counted and logged separately from `skipped`, at -INFO. Conflating them would fire a WARN about a missing drive bind for an app behaving exactly as -asked — and a silent correct path is how an inert seam hides. - -**No new privileged verb**, as required: `StopStack`/`StartStack` are plain `docker compose` execs in -the controller's own container, and the boot path's only agent calls are read-only (`agent.Disks`). -The agent sudoers/wrapper set is untouched — nothing in it relates to app start/stop. - -### Red-proof — RUN - -Dropped `hasContainers` from the predicate: +The catalog change arrived through the **normal git-sync**, not a hand-copy: ``` ---- FAIL: TestShouldRecreateOnBoot - R-55 customer-stopped drive app (zero containers): shouldRecreateOnBoot = true, want false - R-55 gate-stopped app (zero containers; Return branch owns it): shouldRecreateOnBoot = true, want false ---- FAIL: TestRecreateDriveBackedApps_HonoursCustomerStop - recreated=[romm immich], want exactly [romm] — a customer-stopped app must never be restarted +$ grep -m1 '^lifecycle:' /opt/docker/stacks/plant-it/.felhom.yml +lifecycle: abandoned ``` -`recreated=[romm immich]` is the live defect by name. Fix restored → green; `git diff` clean. +| check | result | +|---|---| +| plant-it on the Alkalmazások page | **0 occurrences** (control app `bookstack`: 10) | +| direct `POST /api/stacks/plant-it/deploy` | **HTTP 409** `{"ok":false,"error":"Ez az alkalmazás jelenleg nem telepíthető."}` | +| `/apps/plant-it` | 200, carries the notice, **no Telepítés button** | +| `/apps/bookstack` (control) | 200, no badge, unaffected | -## Gates and deploy verification +Method: the exact endpoints the UI calls, authenticated as the customer (no browser on DooPlex). -`go build ./... && go vet ./... && go test ./...` — **green**, whole module. +## 3. Red-proofs — all five run + +| # | pre-fix shape restored | observed failure | +|---|---|---| +| 1 | remove the API deploy gate | wiring test: *"deployStack never calls Meta.CanInstall() — the lifecycle gate is INERT"* | +| 2 | drop `st.Deployed \|\|` from the catalog filter | `"plant-it-run" must be listed`, `"oldapp-run" must be listed` — a customer's running app vanishes from their own page | +| 3 | remove the badge line from stacks.html | *"a deployed abandoned app must carry the Nem karbantartott badge"* | +| 4 | make orphan detection lifecycle-aware | `catalog set = map[bookstack:true]` — both withdrawn apps read as orphans | +| 5 | restore the pointer receivers (§4) | `can't evaluate field IsAbandoned in type interface {}` on every lifecycle value | + +The wiring test walks the **AST**, not `strings.Contains`, because a commented-out call still +contains the string; it also asserts the gate precedes `DeployStack`, since a gate that fires after +the mutation is not fail-closed. + +## 4. A defect I shipped in v0.158.0 and caught within the hour + +**`/apps/` returned HTTP 500 for EVERY app**, not only withdrawn ones. + +`EffectiveLifecycle` / `CanInstall` / `IsAbandoned` were declared with **pointer receivers**. +`appDetailHandler` passes `data["Meta"] = found.Meta` — a `stacks.Metadata` VALUE inside a +`map[string]interface{}` — and html/template cannot call a pointer-receiver method on a +non-addressable value: ``` -gitea.dooplex.hu/admin/felhom-controller:0.157.0 | Up (healthy) +executing "app_info" at <.Meta.IsAbandoned>: can't evaluate field IsAbandoned in type interface {} ``` -**Not an inert seam** — the new code is verified present in the *running* binary, not just the image: -`grep -a -c "stopped them on purpose" /usr/local/bin/felhom-controller` → `1` inside the live -container. This check exists because three inert-seam defects shipped fully-green in three days. +Fixed in v0.158.1 with value receivers and the reason recorded at the declaration so it is not +"tidied" back. -### The live reboot leg is NOT done, and that is a deliberate stop +**Why every gate missed it, which is the transferable part.** It compiles. `go vet` is silent. All +of v0.158.0's tests passed — because none of them rendered `app_info`. The catalog-page tests +exercised the *funcmap* route (`lifecycleBadge .Meta`), which takes a value and works with either +receiver. **A template method call is only ever checked when that template actually runs**, so a +render test is not optional coverage for template-facing methods — it is the only coverage. -Proving R-55 end to end needs a drive-backed app stopped from the UI followed by `pct reboot 9201`. -9201 has two drive-backed apps (calibre-web, immich) on `hdd_1`, so the fixture is available — but the -demo box is **at a remote site until ~2026-08-02**, and rebooting it unprompted is not a call CC -should make (the 2026-07-20 dhclient incident cost 1h15m from a smaller action). **One operator-present -STOP away.** Until then R-55 is *shipped and wired*, not *proven live*; the capability-map row says so -in those words. +`TestAppInfoRendersForEveryLifecycle` now renders the real template through the production tree with +the handler's exact data shape — `"Meta"` as a value in a `map[string]interface{}`, deliberately not +a pointer, because a pointer would hide the bug the test exists to catch. -## v0.157.1 — `.gitignore` anchor (TASK-E Part 5) +I found this while live-validating, not from the suite. That is the honest sequence. -`controller/.gitignore` carried a bare `controller`, which git matches against **directories** too, so -it also matched `cmd/controller/`. Two opposite failure modes, both manufacturing inert seams: -ripgrep silently skipped `cmd/controller/main.go` (a search for a setter's caller returns nothing and -reads as "unused"), and new files there needed `git add -f` or were never committed. +## 5. Part 3 — R-55 live leg, PROVEN -Anchored to `/controller` + `/controller.exe`. Verified **both** directions: `git check-ignore` no -longer matches `cmd/controller/main.go` and `rg` now finds it; a built binary at the module root is -still ignored. `git ls-files` confirms nothing had been hiding untracked behind the old rule. +Operator-present, 2026-07-21. Fixture on 9201 (guest clock = UTC, host = CEST): -## Observations +| time (UTC) | event | +|---|---| +| 14:28:52 | **immich STOPPED through the real UI endpoint** `POST /api/stacks/immich/stop` → `compose down` | +| — | pre-state verified: immich **0 containers**, calibre-web 1 running (drive-backed), bookstack 2 running (not drive-backed) | +| 14:29:14 | `pct reboot 9201` | +| 14:29:28 | `[gate] boot 1784525102-11906045: waiting (≤2m0s) for live drive bind(s) [/mnt/felhom-drives/hdd_1]` | +| 14:29:28 | `[gate] … live bind confirmed — recreating drive-backed app calibre-web (state=stopped) onto /mnt/felhom-drives/hdd_1` | +| 14:29:29 | `[gate] … 1 drive-backed app(s) left stopped — zero containers means the customer stopped them on purpose` | +| 14:29:32 | `[bootrecon] Boot reconciliation: no boot-orphaned apps (nothing to start)` | -- `TestShouldRecreateOnBoot`'s old comment argued explicitly that a state filter would be a - regression — correct, and still true. The comment is now expanded to say why `hasContainers` is a - *different* axis, so the next reader does not "fix" it back. -- The gate calling `StopStack` before `StartStack` on every recreate is what erases the container - evidence, and is also why R-52's boot-orphan sweep only ever sees non-drive-backed apps. R-52's - practical scope on a real box remains narrower than its brief assumed; unchanged by this fix. +End state: **immich 0 containers / stopped**, calibre-web 1 running, bookstack 2 running. **Zero +alerts** (the only `app_start_failed|deadapp` grep hit was the scheduler registering its job). +Reboot → steady state in **~15 seconds**. + +**immich is absent from the recreate list.** Hours earlier, the identical fixture brought it back +RUNNING — that is the defect R-55 fixed, and this is the same experiment with the opposite result. +The `left stopped` INFO line fired in production for the first time, so the honoured path is +observable rather than silent. + +## 6. Gates + +`go build ./... && go vet ./... && go test ./...` — **green** (both versions). All five template +gates pass (`template_id`, `emoji`, `mojibake`, `app_row_dedup`, `native_confirm`). + +## 7. Observations + +1. **I pushed a commit that does not build.** A `git stash`/`stash pop` used to compare + `reuse_refs_check.py` output silently discarded the staged index, so the first lifecycle commit + landed only the new test files. Fixed by a follow-up commit rather than a force-push (rewriting + pushed history is worse than one bad commit in the log). **Lesson: never `git stash` mid-staging.** +2. `REUSE.md` cross-repo citation drift is pre-existing and improved: 8 broken paths → **5**. I fixed + the three that were plain prefix errors; the rest cite abbreviated paths the checker resolves + against the repo root. +3. The `hidden` state currently has no user in the catalog. It is implemented and tested, but only + `abandoned` is exercised end to end.