From a96226a8ae417a10172a1b9e6aa3c9f58458a030 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Tue, 21 Jul 2026 15:34:13 +0200 Subject: [PATCH] docs(report): R-55 scope correction, spike-lite finding, red-proof and deploy verification --- REPORT.md | 313 +++++++++++++++--------------------------------------- 1 file changed, 86 insertions(+), 227 deletions(-) diff --git a/REPORT.md b/REPORT.md index dd2d270..12d66a1 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,263 +1,122 @@ -# REPORT — TASK-D Parts 1–2: dead-primary alerting (R-51) + boot reconciliation (R-52) +# REPORT — R-55 boot-gate stop-honesty (TASK-E Part 2), 2026-07-21 -**Controller v0.155.0 → v0.156.0.** Session 2026-07-21, on DooPlex as `kisfenyo`. -**Status: SHIPPED, deployed via the floor save, and STOP-1 RAN — both legs passed (§6).** P1 is -answered (§4). The validation also surfaced a real pre-existing gap, filed as **R-55** (§6). Nothing -below claims live behaviour that was not observed. +**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.** ---- +## Scope correction — this part was specified against the wrong repo -## 1. Baselines (re-confirmed at session start) +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. -| Repo | `main` @ start | clean | now | -|---|---|---|---| -| felhom-controller | `0f9b29a` | yes | `285dd10` (v0.156.0) | -| felhom-agent | `08b55a1` | yes | `98adb72` (v0.92.1) | -| felhom.eu | `50a7ffa` | yes | docs commit (see §9) | +**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. -Image: `gitea.dooplex.hu/admin/felhom-controller:0.156.0`, pushed and **never hand-deployed** — the -operator's floor save deployed it, which banked the R-23 single-fire datapoint for free: -`update successful (0.155.0 → 0.156.0)` then `Current version 0.156.0 is up to date`. 9201 runs -**0.156.0**. +**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. ---- +## The defect -## 2. The defect R-51 actually fixes (the ROADMAP row was wrong) +`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 row says aggregation classifies a dead-primary stack `unhealthy`, and that `IsDownState`'s -deliberate `unhealthy` exclusion is therefore what suppresses the alarm. **That is not what the code -did.** `aggregateState`'s final branch was: +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. -```go -// Mix (some running, some stopped) — report as running (partial) -if running > 0 { return StateRunning } -``` +## Spike-lite: a truthful source already existed — nothing was invented -So the immich stack read **running**, not unhealthy. `IsDownState` was never consulted about -`unhealthy` at all, and the constraint the row protects was never in tension with the fix. The -correction is recorded in the ROADMAP row itself. +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: -**The fix.** New `StateDegraded`. The mixed branch asks each DOWN member for its docker restart -policy: `always` / `unless-stopped` → docker was supposed to be keeping it up → the stack is -**degraded**; `no` / `on-failure` → a finished one-shot init/migrate container → benign, stack stays -running. `IsDownState` gains `degraded` **and nothing else** — `downstate_test.go` is untouched and -green, which is the fix-3 contract. +- `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. -**An unreadable policy counts as supervised — fail-CLOSED**, deliberately the opposite of -`IsDownState`'s documented fail-open. They are different questions: there the *state* is ambiguous, -here a member is known dead and only its excuse is missing. P2 backs it (§4). +That is exactly `bootrecon.isBootOrphan`'s `existing-Exited vs absent` distinction. The gate now +additionally requires `len(Stack.Containers) > 0`. -**Blast radius, decided per call site** (`StateDegraded` is a stack-level aggregate and never a -container state, so container-level code is untouched by construction): +**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. -| Question the caller asks | Sites | Degraded counts as | -|---|---|---| -| "are there live containers to stop?" | quiesce `RunningAppStacks`, delete stop-first guard ×2, export `IsStackRunning`, telemetry, health-probe eligibility | **up** | -| "is this app working?" | dashboard counter, stopped filter, dead-app banner + `app_start_failed` | **down** | -| "did this come back healthy?" | `waitHealthy` (migration), post-restore `RefreshAndIsRunning`, integrations readiness | **not healthy** (left strict) | +**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. -The health probe can no longer mask it: the probe result only ever overrides `StateRunning`, so a -failing probe on a degraded stack cannot downgrade it to `unhealthy` (which would silence the alarm -again). Asserted by the wiring test's second refresh. +**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. -## 3. R-52 — the recovery half +**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. -`internal/bootrecon`, one bounded sweep 5 s after startup (after the quiesce recovery, so the two -never race for the same stack): at most **2 attempts, 30 s apart**, then it stops and the alarm owns -the problem. Never a loop. `compose up -d` exits 0 on a crash-loop, so success is decided by a fresh -`RefreshStatus`, never by a nil error. +### Red-proof — RUN -**The safety argument is the container gate.** The UI's Stop is `compose down`, which REMOVES the -containers; a boot interruption leaves them behind as `Exited`. So "deployed, has containers, and -they are down" is the boot-orphan signature, and a zero-container stack is never touched. - -Worst case 5 s + 30 s = 35 s < the 90 s `deadAppBootGrace`, so a successful recovery never alerts -and a failure alerts honestly. `TestBootReconcileFitsInsideTheBootGrace` asserts that arithmetic -instead of leaving it in a comment. - ---- - -## 4. Phase-0 probes - -**P2 (R-51 one-shot census) — DONE, and the answer is clean.** All **53** catalog templates, -**78** services: **every single one is `unless-stopped`**. Zero services with `no` / `on-failure` / -absent. (The other 30 compose files under `output/` are generated artifacts, not templates.) So the -restart-policy guard protects a case that does not exist in today's catalog — it is there for future -templates and for the fail-closed decision above, and it costs one cached `docker inspect` per down -member of a mixed stack. - -**P1 (why `unless-stopped` did not resurrect immich/calibre-web) — ANSWERED during STOP-1, and the -F5 hypothesis is CONFIRMED.** It was never made a blocker (the reconciliation is correct either -way), and it fell out of the R-52 leg for free, as predicted. bookstack carries -`restart=unless-stopped`; the Docker daemon came up at ~10:53:15Z; the container's `StartedAt` is -**`10:53:28.05Z`, the exact moment `bootrecon`'s `StartStack` returned**. Docker did not bring it -back — a container stopped before shutdown is recorded user-stopped and stays down. Details in §6. - -**P3/P4** were agent-side — see `felhom-agent/REPORT.md`. - ---- - -## 5. Tests and red-proofs - -Green gate: `go build ./... && go vet ./... && go test ./...` — **all 25 packages ok** (full run, -`-count=1`; `internal/backup` alone takes 174 s). - -New: `internal/stacks/degraded_test.go` (8 cases incl. two production-path tests), -`internal/bootrecon/bootrecon_test.go` (8 cases), `cmd/controller/bootrecon_wiring_test.go` (4). - -| # | Red-proof | Mutation | Result | -|---|---|---|---| -| A | dead primary must alert | mix branch → `return StateRunning` | **FAIL ×3** — `aggregateState = "running", want "degraded"`; both production-path tests report `immich state = "running"`. Restored, green. | -| C | the boot sweep must be wired | `go runBootReconcile(...)` commented out in `main()` | **FAIL** — `func main() no longer starts the R-52 boot reconciliation`. Restored, green. | -| D | a user-stopped app must stay stopped | zero-container gate dropped from `isBootOrphan` | **FAIL ×2** — `StartStack("jellyfin") called 1 times, want 0 — a deliberate Stop must survive a reboot`. Restored, green. | - -**A finding from red-proof C, worth more than the fix it guards.** That wiring test was first -written as `strings.Contains(src, "go runBootReconcile(ctx, stackMgr, logger)")`. Its red-proof -**passed** — because commenting the call out leaves the string in the file. A test whose own -red-proof cannot fail it is decoration. Rewritten as an AST walk over `func main()`'s body for a -`GoStmt` calling `runBootReconcile`; the red-proof then failed as it must. Recorded in REUSE.md and -promoted into `documentation/PROMPT-TEMPLATE.md` §10 (Part-5 rider). - -**Seam discipline (§9 rule 6).** Both features have a production-path test: -- R-51: `TestRefreshStatus_WiresDegradedThroughTheRealPath` drives the whole real chain — - `RefreshStatus` → `docker ps` → `aggregateState` → `docker inspect` — through a new `execFn` seam - at the process boundary, and asserts the inspect COUNT (exactly 1: only the down member of the - mixed stack, never the healthy stack; still 1 after a second refresh, proving the cache). -- R-52: the sweep is asserted from `package main`, plus the AST wiring test above. - ---- - -## 6. Live validation — STOP-1 RAN, both legs PASSED (operator-present) - -Method: the exact endpoints the UI calls, driven from inside guest 9201 through traefik -(`--resolve felhom.demo-felhom.eu:443:127.0.0.1`, session cookie + CSRF token), plus container-level -truth from `docker ps -a` and the controller's own log. No browser (none on DooPlex). The session -cookie jar was deleted from the guest afterwards. - -**Deploy first, per the resolved ordering:** the operator saved the floor → `0.156.0`, the box -self-updated, and the R-23 datapoint came free — -`[selfupdate] Post-update startup: update successful (0.155.0 → 0.156.0)` then -`Current version 0.156.0 is up to date`: **one swap, no re-fire.** - -### Leg A — R-51, the dead primary (12:50:40 → 12:51:52 CEST) - -| Time | Event | -|---|---| -| 12:50:40 | `docker stop immich-server` → `exited | restart-policy=unless-stopped`; the three helpers stay `running` — the exact F4 shape | -| **12:50:53** | **stack state = `degraded`** (13 s). `immich-server -> exited | Exited (143)`, three helpers `running`. This read `running` for 18 hours on 2026-07-20 | -| 10:51:11Z | **exactly ONE** `Event pushed: app_start_failed (warn) — Telepített alkalmazás nem fut: Immich` — single-fire verified by `grep -c`, = 1 | -| — | Dashboard: banner *"Telepített alkalmazás nem fut: Immich (degraded)"* + link *Rendszermonitor*, and the new „Részlegesen leállt" label present in the rendered page | -| 12:51:37 | `docker start immich-server` | -| 12:51:52 | state back to `running`; **banner self-cleared** (state-based, as designed) | - -The `(degraded)` in the banner is the English-state wart predicted in §7 — visible now, pre-existing, -still a copy decision rather than a bug. - -### Leg B — R-52, the boot orphans (12:53:07 CEST reboot) - -Fixture: `docker stop` on **bookstack** (2 containers) and **calibre-web** (containers left in -place); **UI Stop** on **immich** via `POST /api/stacks/immich/stop` (`compose down` → 0 containers). -Then `pct reboot 9201`. +Dropped `hasContainers` from the predicate: ``` -10:53:22Z [bootrecon] Boot reconciliation: 1 boot-orphaned app(s) found: [bookstack] — up to 2 attempt(s) -10:53:28Z [bootrecon] attempt 1/2: started "bookstack" (took 6.0s) -10:53:28Z [bootrecon] complete: 1 app(s) recovered in 1 attempt(s): [bookstack] +--- 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 ``` -**Zero `app_start_failed`** since the reboot — a successful recovery inside the boot grace is silent, -which was the design claim. And the quiet path is observable on the healthy box too: the earlier -0.156.0 boot logged `Boot reconciliation: no boot-orphaned apps (nothing to start)`, so "nothing to -do" and "never ran" are distinguishable in production, not just in a test. +`recreated=[romm immich]` is the live defect by name. Fix restored → green; `git diff` clean. -### P1 — answered, and the F5 hypothesis CONFIRMED +## Gates and deploy verification -bookstack carries `restart=unless-stopped`. The Docker daemon came up at ~10:53:15Z. The container's -`StartedAt` is **`10:53:28.05Z` — the exact moment `bootrecon`'s `StartStack` returned.** Docker's -own restart policy did **not** resurrect it: a container stopped before shutdown is recorded -user-stopped and stays down across the reboot. Only R-52 brought it back. That is the F5 hypothesis -the audit could not test, settled with direct evidence and at zero extra cost. - -### The finding this leg produced — new R-55 (it is the opposite of what the leg set out to prove) - -Leg B was designed to show that a customer's deliberate Stop survives a reboot. **It does not — for -any drive-backed app.** immich, stopped from the UI seconds earlier, came back **running**: +`go build ./... && go vet ./... && go test ./...` — **green**, whole module. ``` -10:53:18Z [gate] boot …: live bind confirmed — recreating drive-backed app immich (state=stopped) onto /mnt/felhom-drives/hdd_1 -10:53:18Z [stacks] Stopping stack: immich → Starting stack: immich +gitea.dooplex.hu/admin/felhom-controller:0.157.0 | Up (healthy) ``` -`internal/web/intermediary.go`'s boot bind gate selects on `cfg.Deployed && HDD_PATH` alone and runs -`StopStack` + `StartStack` on every match; the stack's `state` is passed in **for the log line only** -and never consulted. calibre-web went the same way; bookstack, not drive-backed, fell through to -R-52 — which is why the sweep found exactly one orphan. +**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. -**R-52 is not implicated.** Its own gate behaved exactly as specified: immich, at zero containers, -was never a candidate. But two things follow and need an operator ruling rather than a drive-by fix -(the gate exists to fix a real bind-ordering bug and must keep doing that): +### The live reboot leg is NOT done, and that is a deliberate stop -1. A customer's Stop is silently undone on drive-backed apps at every reboot. -2. **R-52's practical scope on a real box is narrower than the brief assumed** — the gate reaches - every drive-backed app first, so the boot-orphan sweep only ever sees the remainder. +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. -Filed as **R-55**; likely shape is that the gate should recreate only apps that were RUNNING at -shutdown, i.e. the same running/stopped distinction R-52 already draws. +## v0.157.1 — `.gitignore` anchor (TASK-E Part 5) -## 6b. What was NOT claimed live +`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. -**Nothing beyond the above.** What is verified without the box: the full green gate, all four design gates -(`template_id_gate.py`, `emoji_gate.py`, `native_confirm_gate.py`, `offbox_rename_gate.py` — all -OK), and the image build+push. +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. ---- +## Observations -## 7. UI surfaces (Hungarian) - -New state label „**Részlegesen leállt**", `warn` colour token, `◑` icon (shared with unhealthy), -counted with the stopped apps, filtered into the stopped bucket, and `routeUnpublished` → true (when -the dead member is the routed one Traefik withholds the route and the URL 404s — the card must not -imply the app is reachable). **No new customer strings were needed for the alarm itself**: the -existing `app_start_failed` sentence („Telepített alkalmazás nem fut: …") is reused unchanged, which -is why R-51 ships without touching the notifier. - -One honest wart carried forward, not introduced: the dead-app banner appends the raw state in -English — it will now read `(degraded)` where it read `(stopped)`. Pre-existing behaviour; changing -it is a copy decision, not part of this fix. - ---- - -## 8. A workspace trap found the hard way (worth more than this task) - -`controller/.gitignore` line 7 is `controller` — intended for the built binary. It also matches the -**directory** `controller/cmd/controller/`. Two consequences, both live today: - -1. **ripgrep (and therefore the Grep tool) silently skips `cmd/controller/main.go`** — the entire - production wiring file. Early in this session that produced a false "the whole fix-3 dead-app - path has no production caller" reading — exactly the inert-seam conclusion this task is about. - `grep -rn --no-ignore` / plain `grep -rn` (without `--include`) shows it. -2. **New files under `cmd/controller/` need `git add -f`.** `bootrecon_wiring_test.go` was refused - by a plain `git add` — as part of a multi-path add, i.e. quietly. A wiring test that never - reaches the repo is the same defect class it exists to prevent. - -Recorded in the commit message and REUSE.md. Worth a `.gitignore` fix (anchored `/controller`) as -its own XS change — deliberately not smuggled into this task. - ---- - -## 9. Deliverables - -- `285dd10` — v0.156.0 code + tests + CHANGELOG + REUSE + README. -- Image `felhom-controller:0.156.0` pushed; **not deployed**. -- Docs in `felhom.eu`: ROADMAP R-51/R-52 → SHIPPED (with the corrected R-51 diagnosis), new R-54 - row, a new capability-map row (IMPLEMENTED — not PROVEN-LIVE, the live legs have not run), and the - two Part-5 riders. - -## 10. Operator actions outstanding - -1. **STOP-1** (controller legs) — needs 0.156.0 live; see the ordering question in - `felhom.eu/REPORT.md` §5. -2. **STOP-3** floor → `0.156.0`, which is also what deploys it. -3. P1's answer falls out of STOP-1's reboot leg for free — worth recording either way. +- `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.