297 lines
20 KiB
Markdown
297 lines
20 KiB
Markdown
# REPORT — v0.189.0: the box stops guessing what the customer wanted (R-166 / decision D-b)
|
|
|
|
**Overwritten** per the standing rule. The prior contents (the R-168 CI workflow, same day) have
|
|
their durable record in `CHANGELOG.md`.
|
|
|
|
**Session:** 2026-08-02 · **Repos touched:** `felhom-controller` (v0.188.0 → **v0.189.0**),
|
|
`felhom.eu` (docs only — **no hub change, no hub version bump**) · **Live target:** demo box
|
|
`felhom-pve` guest **9201** · **No STOP point; nothing destructive; no customer data touched.**
|
|
|
|
---
|
|
|
|
## 1. Confirmed baselines — matched §1 exactly on arrival
|
|
|
|
| Repo | `main` @ arrival | Expected (§1) | Match |
|
|
|---|---|---|---|
|
|
| `felhom-controller` | `e7c44c0e0f91d21d92b1dafda3979bf382182c1f` | `e7c44c0e0f91` | ✅ |
|
|
| `felhom.eu` | `c04ea4f6c2b1554f7d5b4f9fbe7b0066c37b4ea1` | `c04ea4f6c2b1` | ✅ |
|
|
|
|
Both trees clean (`git status --porcelain` empty, `HEAD == origin/main`) before the build.
|
|
|
|
## 2. The four §4.9 facts, re-confirmed at source — none had drifted
|
|
|
|
| Fact | Confirmed at | Result |
|
|
|---|---|---|
|
|
| `AppConfig` has no desired-run field; `Deployed` means *installed* | `internal/stacks/deploy.go:97-107` — five fields: `Deployed`, `DeployedAt`, `Env`, `LockedFields`, `EmailEnabled` | **Held** |
|
|
| `StopStack` runs `compose down` (removes containers) | `internal/stacks/manager.go:1056` → `m.composeExec(dir, "down")` | **Held** |
|
|
| `DumpAppVolumesSafe` stops/dumps/starts with no `defer`, no marker, no journal | `internal/backup/backup.go:682-707` — plain statements, no `defer` anywhere | **Held.** Same shape at `internal/backup/offbox_reconstitute.go:279` and `internal/appexport/export.go:232` |
|
|
| `aggregateState` already walks every container; any unhealthy/mixed result wins | `internal/stacks/manager.go:~782` | **Held — D-b's every-container requirement was already met and was NOT re-implemented** |
|
|
|
|
**One fact the spec did not list, and it mattered more than any of them:** `SaveAppConfig`
|
|
(`deploy.go:742`) rebuilt `AppConfig` from a **five-field struct literal**. Adding a sixth field
|
|
without noticing would have dropped `desired_state` on **every** save, across **nine** call sites —
|
|
see §12.
|
|
|
|
## 3. Files created / modified
|
|
|
|
**Created**
|
|
- `controller/internal/stacks/desiredstate.go` — constants, `SetDesiredState`, `DesiredStateOf`, `BackfillDesiredState`, `isObservedUp`
|
|
- `controller/internal/backup/appstop_marker.go` — `AppStopGuard`, `AppStopMarker`, `AppStopRecovery`, reasons
|
|
- `controller/internal/stacks/desiredstate_test.go`, `controller/internal/bootrecon/desiredstate_test.go`, `controller/internal/backup/appstop_marker_test.go`, `controller/internal/api/desiredstate_intent_test.go`, `controller/cmd/controller/appstop_wiring_test.go`
|
|
|
|
**Modified**
|
|
- `controller/internal/stacks/deploy.go` — `DesiredState` field; **`SaveAppConfig` → copy-and-overlay**; `DeployStack` records `running`; `UpdateOptionalConfig`'s redeploy branch records `running`
|
|
- `controller/internal/api/router.go` — `desiredStateForAction` + the intent write in `actionStack`
|
|
- `controller/internal/bootrecon/bootrecon.go` — `isBootOrphan` rewritten on intent; package doc updated
|
|
- `controller/internal/backup/backup.go` — `appStop` field, guard construction, `SetAppStopGuard`/`AppStopGuard`, `DumpAppVolumesSafe` bracketed
|
|
- `controller/internal/backup/offbox_reconstitute.go` — `Begin` + a `restartStack` closure covering all four bring-up paths
|
|
- `controller/internal/appexport/export.go` — consumer-side `appStopGuard` seam + `SetStopGuard`
|
|
- `controller/cmd/controller/main.go` — guard construction + `Recover` before the bootrecon goroutine, backfill, `NotifyBackupFailed` reporting, `exportStopGuard` adapter, `.fab` restore records `running`
|
|
- `CHANGELOG.md`, `CONTEXT.md`, `REUSE.md`, `controller/README.md`
|
|
- `felhom.eu`: `CONTEXT.md` (S-12), `CLAUDE.md`, `STATUS.md`, `documentation/architecture/00-capability-map.md`, `documentation/architecture/02-controller-module-map.md` (**new §0a — the S-1 contract**), `documentation/backlog/OPEN-ITEMS.md`, `documentation/backlog/ROADMAP.md`
|
|
|
|
## 4. Commits pushed to `main`
|
|
|
|
| Repo | Commit | Subject |
|
|
|---|---|---|
|
|
| `felhom-controller` | `dbcb306` | v0.189.0 — desired state + the app-stop crash marker (R-166 / D-b) |
|
|
| `felhom.eu` | *(docs commit, this session)* | docs: R-166 shipped — S-1 contract, capability map, backlog, STATUS |
|
|
|
|
Direct to `main`, no branches, no `Co-Authored-By`. The pre-push hook ran the gate entry point
|
|
(`--fast`) and passed; **`--no-verify` was NOT used.**
|
|
|
|
## 5. Tests and red-proofs
|
|
|
|
**27/27 packages green before and after; +37 test functions across 5 packages.**
|
|
Full `go build ./... && go vet ./... && go test ./...` → **rc=0**, run to completion twice.
|
|
Test run and commit were **never** combined in one command (standing rule 1).
|
|
|
|
Every red-proof below was **observed FAIL, then the mutation was reverted and a control run passed.**
|
|
|
|
| # | Test | Mutation applied | Result |
|
|
|---|---|---|---|
|
|
| **B** | `TestReconcile_DesiredRunning_ZeroContainers_IsRecovered` + `TestIsBootOrphan_DecisionTable` | restored `len(s.Containers) > 0 && IsDownState(...)` in the `DesiredStateRunning` branch | **FAIL** ✅ |
|
|
| **C** | `TestReconcile_LegacyNoDesiredState_BehavesExactlyAsBefore` + `..._AppConfigPresentButFieldAbsent...` | made the `default:` (absent) branch return `len == 0 \|\| IsDownState(...)`, i.e. absent treated as running | **FAIL** ✅ — *"a LEGACY app with no recorded intent and zero containers was started 1 time(s)"* |
|
|
| **D** | `TestBackfillDesiredState_RunningIsRecorded_AmbiguousIsLeftAlone` + `..._NeverWritesStopped...` | deleted the `if !isObservedUp(s)` guard | **FAIL** ✅ — *"exited (state=exited) was backfilled to \"running\""* |
|
|
| **E** | `TestDumpAppVolumesSafe_MarkerCoversTheWholeStopStartWindow`, `..._Interrupted_RecoveryBringsTheAppBack`, `..._FailedRestartKeepsTheMarker` | deleted the production `m.appStop.Begin(...)` call from `DumpAppVolumesSafe` | **FAIL** ✅ (3 tests) |
|
|
| **H** | `TestSaveAppConfig_PreservesEveryKnownFieldAcrossLoadSave` + `TestSetDesiredState_PersistsStoppedToDisk` | restored the field-by-field `saveCfg := AppConfig{...}` literal | **FAIL** ✅ — *"desired_state was LOST across load→save"* |
|
|
| **§8.2** | `TestActionStack_RecordsIntentBeforeActing` | moved the intent write **below** the action switch | **FAIL** ✅ |
|
|
| **I (seam)** | `TestMainWiresAppStopRecovery` + `TestAppStopRecoveryPrecedesTheBootReconciler` | **commented out** `appStopGuard.Recover()` in `main.go` | **FAIL** ✅ — and `grep -c "appStopGuard.Recover()" main.go` still returned **1**, i.e. a `strings.Contains` test would have passed. The AST walk is what bites. |
|
|
|
|
### The hollow test I caught and fixed
|
|
|
|
Scenario E's first version called `m.appStop.Begin(...)` itself and then recovered — and it
|
|
**PASSED under red-proof E** while its two siblings failed. It proved the marker type worked, not
|
|
that `DumpAppVolumesSafe` uses it. Rewritten to drive the real function with a simulated hard abort
|
|
(`GetDockerVolumes` panics, unwinding past the restart *statement* — which is the point, since a
|
|
`defer` is not crash-safety). It then failed the same red-proof correctly.
|
|
|
|
## 6. The §8.1 table — every row proven by test
|
|
|
|
`TestIsBootOrphan_DecisionTable` (10 sub-tests, all green; the whole table fails under red-proof B):
|
|
|
|
| `desired_state` | containers | state | Expected | Sub-test |
|
|
|---|---|---|---|---|
|
|
| `stopped` | 0 | stopped | not an orphan | `stopped/no containers` ✅ |
|
|
| `stopped` | 2 | exited | not an orphan | `stopped/down containers` ✅ |
|
|
| `stopped` | 2 | running | not an orphan | `stopped/running` ✅ |
|
|
| `running` | 0 | — | **ORPHAN** | `running/no containers` ✅ |
|
|
| `running` | 2 | exited | **ORPHAN** | `running/down containers` ✅ |
|
|
| `running` | 2 | degraded | **ORPHAN** | `running/degraded` ✅ |
|
|
| `running` | 2 | running | not an orphan | `running/up` ✅ |
|
|
| absent | 0 | — | **not an orphan (legacy)** | `absent/no containers` ✅ |
|
|
| absent | 2 | exited | **ORPHAN (legacy)** | `absent/down containers` ✅ |
|
|
| absent | 2 | running | not an orphan | `absent/up` ✅ |
|
|
|
|
Plus `TestIsBootOrphan_ExistingGuardsSurviveTheRewrite` — `Protected`, `Deploying` and `!Deployed`
|
|
still refuse at the strongest desired state, because the rewrite reordered the terms and a reorder is
|
|
how a guard silently disappears.
|
|
|
|
## 7. Build, deploy, gates
|
|
|
|
```
|
|
build.sh 0.189.0 --push → gitea.dooplex.hu/admin/felhom-controller:0.189.0 (145M)
|
|
guest 9201: docker pull → /etc/felhom-controller-image → systemctl restart felhom-controller-bootstrap
|
|
```
|
|
Verified live: `gitea.dooplex.hu/admin/felhom-controller:0.189.0 | Up … (healthy)`.
|
|
`python3 controller/scripts/controller_gates.py` — **all 8 gates OK** (template-id, emoji,
|
|
native-confirm, offbox-rename, app-row-dedup, mojibake, docker-v, reuse-refs: 133 cited paths,
|
|
126 exact / 6 suffix / 1 cross-repo, 0 failures).
|
|
|
|
## 8. Live validation on the demo box — method, and the observables
|
|
|
|
**Method: endpoint-level** (`claude-in-chrome` is not available on DooPlex). The **exact request the
|
|
UI's „Leállítás"/„Indítás" buttons make** was driven — `layout.html:231`,
|
|
`POST /api/stacks/<name>/<action>` with `X-CSRF-Token`, session-authed through traefik. No server
|
|
logic was skipped; only rendering. **The rendered click-through remains the operator's manual check.**
|
|
|
|
Two traps hit, both already in project memory: curl's cookie jar drops `felhom_session` (handled by
|
|
hand, one cookie), and the controller is not published on `:8080` — it sits behind traefik on `:443`
|
|
with `Host: felhom.demo-felhom.eu`. The credential-bearing helper was deleted afterwards (§14).
|
|
|
|
### Flow 0 — the backfill, on four real legacy app.yaml files
|
|
|
|
```
|
|
[INFO] [stacks] desired-state backfill: 4 app(s) recorded as running, 0 left unrecorded …
|
|
```
|
|
`bookstack`, `immich`, `docmost`, `calibre-web` — all deployed, all observed up, all with **no**
|
|
`desired_state` before the upgrade, all converged to `desired_state: running` on disk.
|
|
|
|
### Flow 1 — a customer's Stop survives a restart (Scenario A)
|
|
|
|
```
|
|
action_response={"ok":true,"message":"Stack calibre-web stop completed"}
|
|
[INFO] [stacks] desired state for calibre-web recorded as "stopped" (was "running") ← 16:45:51
|
|
[INFO] [stacks] Stopping stack: calibre-web ← 16:45:51
|
|
```
|
|
The intent line **precedes** the stop line — §8.2 observed live, not only structurally.
|
|
|
|
- `app.yaml` on disk: **`desired_state: stopped`**
|
|
- `docker ps -a --filter name=calibre` → **empty** (`compose down` removed the containers)
|
|
- controller restarted → `[bootrecon] Boot reconciliation: no boot-orphaned apps (nothing to start)`
|
|
- `docker ps -a` → **still empty. The app stayed down.**
|
|
|
|
### Flow 2 — the power-cut shape is recovered (Scenario B — the point of the release)
|
|
|
|
Started from the UI endpoint → `desired_state: running`, `calibre-web Up 8 seconds`. Then the R-157
|
|
shape was created out-of-band: `docker rm -f calibre-web` → `containers_now: 0`, `app.yaml` untouched
|
|
at `running`. Controller restarted:
|
|
|
|
```
|
|
[INFO] [stacks] desired-state backfill: 0 app(s) recorded as running, 0 left unrecorded …
|
|
[INFO] [bootrecon] Boot reconciliation: 1 boot-orphaned app(s) found: [calibre-web] — up to 2 attempt(s)
|
|
[INFO] [bootrecon] Boot reconciliation attempt 1/2: started "calibre-web" (took 0.2s)
|
|
[INFO] [bootrecon] Boot reconciliation complete: 1 app(s) recovered in 1 attempt(s): [calibre-web]
|
|
```
|
|
`calibre-web Up 56 seconds (healthy)`. **Before v0.189.0 this app was invisible to the reconciler.**
|
|
|
|
### Flow 3 — a legacy app.yaml behaves exactly as before (Scenario C)
|
|
|
|
`desired_state` deleted from `calibre-web/app.yaml` (`desired_state_lines: 0`), containers removed
|
|
(`containers: 0`), `deployed: true` kept. Controller restarted:
|
|
|
|
```
|
|
[INFO] [stacks] desired-state backfill: 0 app(s) recorded as running, 1 left unrecorded (state ambiguous — legacy boot behaviour retained)
|
|
[INFO] [bootrecon] Boot reconciliation: no boot-orphaned apps (nothing to start)
|
|
```
|
|
App **not** started; `grep -c "^desired_state" app.yaml` → **0**, i.e. `stopped` was **never
|
|
inferred**. Byte-identical to pre-v0.189.0 behaviour, with a positive observable for both halves.
|
|
|
|
**Not live-validated, deliberately, and stated rather than rounded up:** the app-stop marker's
|
|
recovery leg. It is unit-proven and red-proofed, but nobody killed the controller mid-backup on real
|
|
hardware. The capability map records it as **IMPLEMENTED, not PROVEN-LIVE**.
|
|
|
|
## 9. Every customer-intent entry point found
|
|
|
|
A census of all 14 `StartStack`/`StopStack` call sites was run (`grep -rn --include=*.go`, tests
|
|
excluded). **Four are the customer; ten are machines.**
|
|
|
|
| # | Intent point | Records | Note |
|
|
|---|---|---|---|
|
|
| 1 | `internal/api/router.go` `actionStack` | start/restart/update → `running`; stop → `stopped` | the primary one, named in §1.3 |
|
|
| 2 | `internal/stacks/deploy.go` `DeployStack` | `running` | named in §1.3 |
|
|
| 3 | `internal/stacks/deploy.go` `UpdateOptionalConfig` (redeploy branch only) | `running` | **found, not assumed** — an API endpoint ending in `compose up -d`; the non-deployed branch starts nothing, so it expresses no opinion |
|
|
| 4 | `cmd/controller/main.go` `exportAdapter.SaveEncryptedAppConfig` | `running` | **found, not assumed** — the `.fab` import path, which then starts the app (`appexport/restore.go:461`) |
|
|
|
|
**Confirmed: no machine-initiated caller writes desired state.** The machine callers —
|
|
`quiesce/quiesce.go:494`, `backup/backup.go:688`, `backup/restore.go:55`, `backup/restore_unit.go:252`,
|
|
`backup/tier2_restore.go:177`, `backup/offbox_reconstitute.go:279`, `appexport/export.go:232`,
|
|
`appexport/restore.go:366`, `stacks/migrate.go:707`, `web/intermediary.go:186,430`,
|
|
`web/storage_handlers.go:526`, `bootrecon/bootrecon.go:135` — were each read and none touch it.
|
|
|
|
## 10. Marker coverage — all three sites, none left out
|
|
|
|
| Site | Covered | How |
|
|
|---|---|---|
|
|
| `backup/backup.go` `DumpAppVolumesSafe` | ✅ | `Begin` before the stop; `End` only on a successful restart; cleared on a **failed stop** (nothing was stopped, so nothing is owed) |
|
|
| `backup/offbox_reconstitute.go` | ✅ | `Begin` before the stop; one `restartStack` closure replaces all **four** bring-up paths, so the success path cannot silently skip the clear |
|
|
| `appexport/export.go` | ✅ | consumer-side two-method seam; `main.go`'s `exportStopGuard` supplies the reason, so `backup.ReasonAppExport`'s value exists in exactly one place |
|
|
|
|
**Nothing was left out.** All three share **one** guard over **one** file: `main.go` builds it,
|
|
`backup.Manager.SetAppStopGuard` receives it, `Exporter.SetStopGuard` receives it. A second guard
|
|
over the same path would be one file with two owners — the shape this marker was kept out of
|
|
quiesce's file to avoid.
|
|
|
|
## 11. The operator event path (§2.4) — reachable, used, no new channel
|
|
|
|
**Reachable, but not from where `Recover` runs.** `Recover` must complete before the boot-reconcile
|
|
goroutine (`main.go:~236`) and the notifier is not constructed until `~307`. Rather than wire a seam
|
|
that would fire after the fact — the "built but never wired" shape with four recorded instances here
|
|
— `Recover` **returns** an `*AppStopRecovery` and `main.go` reports it once the notifier exists.
|
|
|
|
It rides the **existing** `backup_failed` event type (`notifier.NotifyBackupFailed`). A new type
|
|
would need the hub's `allowedEventTypes` + `customerMessages` pair changed — a wire change, and this
|
|
release ships **no hub change**. The call is guarded by `if appStopRecovery != nil` so a healthy boot
|
|
pages nobody; `TestMainReportsTheInterruptedOperation` asserts both the call and the guard.
|
|
|
|
## 12. Does `app.yaml` round-trip unknown keys? — MEASURED: **no**
|
|
|
|
`TestSaveAppConfig_UnknownYAMLKeysAreDropped` writes an `app.yaml` containing
|
|
`future_field: keep-me`, loads it, saves it, and reads the file back: **the key is gone.** The
|
|
round-trip goes through the struct, so `yaml.Unmarshal` discards unmodelled keys before
|
|
`SaveAppConfig` ever sees them. Pre-existing, not widened by R-166, and now stated in
|
|
`SaveAppConfig`'s own comment — it is why **every** writer must load-then-save rather than construct.
|
|
|
|
**What WAS fixed is the more dangerous half.** `SaveAppConfig` rebuilt `AppConfig` from a five-field
|
|
struct literal — the **R-100 shape**, which v0.181.0 shipped two live instances of. `desired_state`
|
|
would have been dropped on every save across **nine** call sites, so a customer's Stop would have
|
|
been erased by the next unrelated `app.yaml` write (an email-toggle change, an optional-config edit,
|
|
the encryption migration). Replaced with copy-and-overlay (`saveCfg := *cfg`), safe by construction
|
|
and pinned by `TestSaveAppConfig_PreservesEveryKnownFieldAcrossLoadSave` + red-proof H.
|
|
|
|
## 13. `OPEN-ITEMS.md` rows opened / closed / re-ranked
|
|
|
|
- **R-166 → SHIPPED + PROVEN-LIVE.** Both blocking facts recorded with their answers: (a) the
|
|
crash-safe journal pattern **did** already exist (quiesce, migrate) but covered **none** of the
|
|
app-data path — `DumpAppVolumesSafe` had no marker, no journal and not even a `defer`; (b) the
|
|
SQLite store is reachable and **deliberately unused**, because `metrics.db` is optional by design
|
|
and operational state must not live in a store built to be dropped.
|
|
- **R-157 → mechanism B CLOSED, mechanism A remains** and is now the whole row (READY, S). A is a
|
|
pure timing fix (the sweep observes ~5 s after start and never re-checks). Recorded: **B's fix
|
|
makes A cost more**, since the sweep now has more it could legitimately recover.
|
|
- **R-170 → NEW.** `shouldRecreateOnBoot` (`internal/web/intermediary.go:131`) still ends in
|
|
`&& hasContainers` — R-157 mechanism B, one gate over, for drive-backed apps. **Established free:**
|
|
`grep -ro "R-170\b" documentation/ *.md` → **0 hits**. Not acted on (§16).
|
|
- `ROADMAP.md` gained matching R-166/R-170 rows and an updated R-157; `STATUS.md`'s "an app can stay
|
|
switched off and nothing says so" line was rewritten, not extended.
|
|
|
|
## 14. Teardown
|
|
|
|
**This task provisioned nothing** — no VM, no guest, no scratch app, no storage. The one app used for
|
|
validation (`calibre-web`) already existed and was **restored to its original state**: started
|
|
through the same UI endpoint, `desired_state: running` on disk, `calibre-web Up … (health: starting)`,
|
|
15 containers running on the box, controller healthy on 0.189.0. The credential-bearing helper script
|
|
was deleted from **both** the guest (`/tmp/act.sh` plus its scratch HTML) and DooPlex; `ls` confirms
|
|
`No such file or directory`.
|
|
|
|
## 15. CI
|
|
|
|
| Run ID | `head_sha` | Conclusion |
|
|
|---|---|---|
|
|
| **17** | `dbcb306f` (the controller push) | **success** |
|
|
|
|
Checked by pulling the run list, not by trusting the absence of a failure email — which is exactly
|
|
the check now added to `felhom.eu/CLAUDE.md`'s end-of-session list (§N.6).
|
|
|
|
## 16. Observations — noticed, documented, NOT acted on
|
|
|
|
1. **R-170 (filed).** The drive-backed boot gate still infers a Stop from a container count. Left
|
|
alone deliberately: the task scoped `bootrecon` and named this gate nowhere, and shipping two boot
|
|
behaviour changes under one live validation is one too many. Its consequence is **bounded, not
|
|
zero** — `bootrecon` now recovers the app on intent seconds later, so what is actually lost is the
|
|
gate's *recreate-against-the-rebound-drive* step, which matters when an app needs recreating
|
|
rather than merely starting.
|
|
2. **`app.yaml` cannot carry an unmodelled key** (§12). Pre-existing; documented in-code rather than
|
|
fixed, since fixing it means an inline catch-all map on a struct holding encrypted secrets.
|
|
3. **The `.fab` restore adapter constructs a fresh `AppConfig`** (`main.go`), so it does not benefit
|
|
from copy-and-overlay — it is a genuinely new record. Correct today; noted because any future
|
|
field added to `AppConfig` must be considered there explicitly.
|
|
4. **Part 3 (UI) — N/A: no user-visible string changed.** No template, funcmap, badge, copy or
|
|
notifier-message change. The v0.164.0 `StateStopped` suppression is untouched and still correct:
|
|
an app the customer stopped is still not an alarm, and it is now not a recovery candidate either —
|
|
for a *recorded* reason instead of an inferred one.
|
|
5. **`aggregateState` already satisfies D-b's every-container requirement** and was deliberately not
|
|
re-implemented. The backfill's `isObservedUp` is an allow-list of up-states rather than
|
|
`!IsDownState`, because that negation would call a crash-looping or unreadable stack "up" and seed
|
|
a durable record from it.
|