From 1da2c9c6c6c30fc2edf0da1bbb9355c97c695e81 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Sun, 23 Aug 2026 12:06:45 +0200 Subject: [PATCH] docs(v0.223.0): REPORT, CONTEXT rulings, README severity contract REPORT overwritten: the 1.1 sweep in full (one bad severity, nine legitimate "warn" strings that are healthcheck statuses), the hub manifest's real location since the task's premise was wrong, all five red-proofs with the layer each guard sits at, the live walk in six steps with the hub's own records quoted, and the absent-intent count (0 of 8). Three things are reported that a tidier account would omit: red-proof 5 passed first time because the mutation was INERT; Scenario G was silently refused twice behind an HTTP 200; and the live Scenario A does NOT prove the customer gate, because demo-hp has no prefs row at all. CONTEXT records the severity vocabulary as a ruling with its mechanism, the intent ruling with its three-way handling of unknown, both fences, and two traps worth more than the fixes: a 200 can be a refusal, and a passing red-proof can mean an inert mutation. README: the event table said `app_start_failed | warn` - the defect, written down as if correct. Now `warning`, with the vocabulary contract and who receives what. `disk_critical` also corrected from `error` to `critical`, which is what fillwatch has always sent. --- CONTEXT.md | 56 +++++- REPORT.md | 401 +++++++++++++++++++++---------------------- controller/README.md | 20 ++- 3 files changed, 273 insertions(+), 204 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 13db583..e0761a7 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -7,7 +7,61 @@ > > Ask Claude Code: "Please update CONTEXT.md with what we did today" -Last updated: 2026-08-23 (v0.222.0 — R-384: a dead database raised no alarm; R-383; and R-386 filed) +Last updated: 2026-08-23 (v0.223.0 — R-329: the alarm reached nobody; R-386: ask the field that knows) + +> **2026-08-23 — v0.223.0 (R-329 + R-386), and a defect that only became visible once another was fixed.** +> +> **[RULING] The severity a controller sends is the HUB's vocabulary: exactly +> `{info, warning, error, critical}`.** Anything else is **coerced to `info` at ingest, silently**, and +> `severityNotifies` drops `info` **before both** delivery legs. `app_start_failed` emitted `"warn"`. +> **Measured on the live hub DB: 91 such events stored all-time, ZERO notification rows ever.** +> +> **[FACT] This was the SECOND occurrence, and the first one's comment had recorded the lesson.** +> `DiskAlertKind.Severity` emitted `"warn"` until v0.215.0. **A comment is not a guard** — the guard is +> now an AST walk over the whole controller. **grep cannot do this job:** `"warn"` is a legitimate +> *healthcheck status* in `internal/monitor` and `internal/selftest`; the sweep hit nine such strings +> and exactly one defect. The walk cannot follow a variable, so the **six** dynamic call sites are +> registered by name with the values each can take — **an unlisted limit is not a limit, it is a hole**. +> The guard found two of those six that the hand sweep had missed. +> +> **[FACT] It hid because another defect hid it.** R-384's ordering bug meant `app_start_failed` could +> not fire at all, so a broken severity had nothing to break. **Fixing one defect made another +> reachable** — and the same shape appeared again downstream: the operator cooldown key carries no app +> identifier, so **only the first app-down per hour now e-mails the operator** (R-182's shape, newly +> load-bearing, filed not fixed). +> +> **[RULING] `app_start_failed`: operator always, customer OFF by default.** `processOperator` never +> consults customer preferences, so one word fixed the operator leg and left the customer leg where the +> ruling wanted it. **Deliberately NOT in `operatorOnlyEvents`** — that would make the new toggle +> visible, flickable and structurally incapable of delivering. +> +> **[RULING, R-386] "The customer stopped this" is a RECORD, never an inference.** `aggregateState` +> folds `StateExited` into the stopped counter, so an out-of-band stop and a customer's Stop are +> byte-identical on the Docker side — no state test can separate them. Ask `DesiredState`, which has +> exactly one writer. `Stopped` → no alarm; `Running` → **alarm**; **absent → UNKNOWN, keep the old +> behaviour AND announce it**, because reading absent as "nobody asked" would e-mail about every app +> anyone ever stopped, fleet-wide, on the first cycle after upgrade. **The backfill cannot help — it +> seeds `Running` only from an observed-UP reading.** +> +> **[MECHANISM] `IntentUnknown` + an INFO line naming the apps.** A rule without a mechanism is a wish. +> Measured on `demo-hp`: **0 of 8** deployed apps carry an absent intent. +> +> **[FENCE] Adding a `DesiredState` WRITER is the fenced act; reading is fine.** And `failedRestart` +> must still lift a `Stopped` intent, or F-CRIT-1 re-opens. +> +> **[TRAP, cost three attempts] An HTTP 200 can be a REFUSAL.** The settings save answers 200 while +> rendering the empty-email wipe-guard error. Scenario G's before/after hashes matched twice because +> **nothing was saved**, not because nothing changed. And the email `` spans three lines, so a +> single-line grep reads it empty. **Assert the refusal banner is ABSENT before believing a save.** +> +> **[TRAP] A red-proof that passes may mean an INERT mutation.** `if next <= prev` → `if next < prev` +> in fillwatch changes nothing, because an earlier `if next == prev { continue }` already removed the +> equal case. Check the mutation applied before believing either verdict. +> +> **[RULING] The compound-toggle split's risk was the MIGRATION, not the split.** A save whose event +> SET is unchanged now stores the existing slice **verbatim**, so byte-identity is by construction — +> without that guard the defaults case reorders, and the red-proof caught it. + > **2026-08-23 — v0.222.0 (R-384 + R-383), and a bigger hole found by a measurement that was told not to fix it.** > diff --git a/REPORT.md b/REPORT.md index 5a6893d..67bae3b 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,267 +1,266 @@ -# REPORT — controller v0.221.1 (record) + v0.222.0 (R-384, R-383) +# REPORT — controller v0.223.0 (R-329, R-386, and the compound toggles) -**Session 2026-08-23, UNATTENDED. Live leg on `demo-hp` (Tier 0, disposable).** +**Session 2026-08-23, UNATTENDED.** Live leg on `demo-hp` (Tier 0), guest 9201. +**No halt condition fired.** Nothing was dropped. -> ## ⚠ HALT DECLARED — §4's measurement found a real defect, filed as R-386, NOT fixed -> -> The task's §4 asked for a measurement and named it a halt condition. It reproduced. -> **A single-container app stopped out of band raises no alarm at all.** Details in §10 below. -> Per §13 the fix was NOT attempted here. Live-walk step 4 (Scenario E) was dropped as a -> consequence — it is item (3) on the task's own drop list. Everything else completed. +## 1. Baselines, and the hub's four numbers as read ---- - -## 1. Baselines used, and the hub's four numbers as read - -| Repo | `main` at start | Verified | +| Repo | at start | at end | |---|---|---| -| felhom-controller | `f7881787f434` | matches the task | -| felhom.eu | `1eb64bec5183` | task said `4e488321bfd1+`; it had moved on | -| felhom-agent | untouched | — | +| felhom-controller | `14137efa` (v0.222.0) | **v0.223.0** deployed | +| felhom.eu | `55274d5e` (hub v0.106.0) | **hub v0.107.0** deployed | +| felhom-agent | `40d857b5` (v0.130.0) | untouched | -**Hub's own numbers, read live from `/configuration` (ClusterIP + Basic auth) 2026-08-23:** +**Hub's four numbers, live from `GET /configuration` before starting:** `golden_version` **0.222.0**, +`agent_version` **0.130.0**, `min_agent` **0.129.0**, controller floor **0.222.0** — all four as the +task predicted. -| Field | Value | -|---|---| -| `golden_version` | **0.221.1** | -| `agent_version` | **0.130.0** | -| `min_agent` | **0.129.0** | -| controller floor (`min_controller_version`) | **0.221.1** | +## 2. Documents read -The task expected golden/floor **0.220.2**; the operator had already vouched **0.221.1** and raised -the floor. Live controller on `demo-hp` at session start: **0.221.1** — so the running version, the -golden and the floor all agreed, and only the RECORD disagreed. That is exactly R-385's shape. +`internal/notify/notifier.go:583-602` (`Severity()`'s doc comment — **it already stated the entire +contract and named both hub locations**), `hub/internal/api/handler.go` (the type rejection and the +severity coercion side by side), `hub/internal/notify/dispatcher.go` (`severityNotifies`, +`ProcessEvent`, `operatorOnlyEvents`, `processOperator`), `internal/stacks/deploy.go` (the +`DesiredState` comment and its one-owner rule), `cmd/controller/main.go` (`classifyRunStates`), +`internal/web/handlers.go` (the compound toggles). -## 2. Architecture documents read +**§2's conditional is answered: the alarm ladder DOES exist** — +`felhom.eu/documentation/architecture/08-alarm-ladder.md`, written last session. It has been extended +here with §6.1 (the severity contract), §7 (the intent test) and §8 (Part 5's direction). -- `documentation/architecture/00-capability-map.md` — its 2026-08-22 paragraph already NAMED R-384 as - an open finding, from the held-app measurement. -- `felhom-controller/internal/stacks/manager.go` `IsDownState` + `aggregateState` + `supervisedPolicy` -- `cmd/controller/main.go` `classifyRunStates` and its three suppressions -- `internal/quiesce/suppress.go`, `internal/bootrecon/bootrecon.go`, `internal/stacks/desiredstate.go` -- `felhom.eu/scripts/golden_currency_gate.py` (all 133 lines) +## 3. The 1.1 sweep — the result in full -**§2's conditional applies and is answered: NO document owned the alarm ladder.** That absence is -reported as a finding, and `documentation/architecture/08-alarm-ladder.md` now owns it (Part N.4). -It is why the ordering defect was legible only by reading one function top to bottom. +**Exactly ONE bad severity in the whole controller: `notifier.go:546`, `"warn"`.** Nothing else. -## 3. Part 0 — the record, pushed ALONE +Verified across Go **and** templates **and** queued-event construction, because the task warned that +reading zero from Go files while the answer sat in a template has produced three wrong conclusions +here: -Exact heading written: +- every `emit(...)` / `PushEvent(...)` literal — one offender, the rest valid; +- `internal/channelhealth`'s classifier (the source of `NotifyAgentChannelDown`'s variable) — all + `"warning"`/`"error"`; +- `debug.html`'s operator-triggerable severity `` +spans **three lines**, so a single-line grep read it as `""`. **Both times the hashes matched — +because nothing was saved, not because nothing changed.** Fixed by asserting the refusal banner is +absent. *A warning beside a success is read as a success.* -`aggregateState` folds `StateExited` into the `stopped` counter, so when every member is down it -returns `StateStopped` — **`StateExited` never survives aggregation**, which is the path the task -suspected and could not find. `classifyRunStates` then whitelists `StateStopped` as a deliberate user -stop. So the comment at `cmd/controller/main.go` — *"An out-of-band `docker compose stop` leaves the -containers present → StateExited → still alerts, which is correct: out-of-band tampering IS -reportable"* — is **false**, and so is the neighbouring I2 claim that a crashing app never comes to -rest at `stopped`. +### Step 6 — Scenario H: a bad severity to the hub ✅ -**Measured:** `privatebin` (1 container, `unless-stopped`) stopped 05:47:35Z. At 05:51:53Z: -`state=stopped`, **9 dead-app scans had run, 0 events, 0 banner lines.** -**Positive control first, per standing rule 3:** `app_start_failed` fired for BookStack at 05:30:14Z -on the same box 17 minutes earlier, so the detector was demonstrably alive. +``` +[WARN] [api] Event from demo-hp: severity "warn" is not in {info,warning,error,critical} — coercing +to "info", which severityNotifies DROPS, so this backup_failed alert will reach NOBODY. Fix the +emitting controller; this event is stored but not routed. +``` -**Scoped honestly:** a genuine *crash* under `unless-stopped` is restarted by Docker and surfaces as -`restarting` → the 5-minute crash-loop path, which does alarm. The silent case is an explicit -out-of-band stop of a stack with no surviving member. +Both the bad POST and an `error` control returned **200** (nothing lost); the control produced **no** +warning. -**Filed as R-386 (OPEN — MEDIUM). Not fixed here, per §12 and §13.** +## 10. The absent-intent count on `demo-hp`, in plain words -## 11. Evidence +**Zero.** All **8** deployed apps carry `desired_state: running`; none is `stopped` and none is absent. +The unknown-intent fallback therefore suppresses nothing on this box today — the population is already +empty on a machine that has been exercised through the interface. It will be larger on a box upgraded +and left alone, which is why the log line exists rather than a one-off count. -`felhom.eu/documentation/audits/DRILL-r384-dead-db-alarm-2026-08-23/evidence/` — 5 gate runs, 4 -red-proof transcripts, 20 live-walk files including two full controller-log windows (1808 and 2139 -lines) pulled off the guest. **Both log windows were copied off before the app was restarted**, per -standing rule 5. +## 11. The dead-branch decision, and the reason -## 12. Teardown, three layers, and the box's end state +**KEPT.** `cmd/hub/main.go` wires `dispatcher.ProcessEvent` **directly** as the +`monitor.EventNotifyFunc` for the staleness, host-staleness and offsite-box checkers — those events +never pass the ingest handler, so for them that line is the only severity guard there is. Deleting it +as "dead" would have removed the live half while the dead half supplied the justification. All 90 +severity literals in `internal/monitor` were verified already valid, so the guard is silent because +the producers are correct. -1. **Guest 9201 / apps** — nothing provisioned. `bookstack-db` restarted and **`bookstack` confirmed - healthy**; `privatebin` restarted and healthy; `docmost` (all 3) healthy. Planted data untouched - throughout — no app was rebuilt, redeployed or restored. -2. **Bake VM** — the drill VM on DooPlex ran the bake and its build guest 9100 is stopped inside it; - the qcow2 reverts to the `virgin` snapshot. No storage was added anywhere, so `pvesm status` has - nothing to compare. -3. **Hub-side record — stated explicitly even though there is none.** No appliance was registered, no - customer created, no config written, no artifact manifest changed. **The hub was READ ONLY** - (`GET /configuration`, `GET /events`). Nothing to discard. +## 12. Evidence -**End state:** `demo-hp` guest 9201 runs controller **0.222.0**, all 8 deployed apps healthy, golden -0.222.0 baked and published but **NOT vouched** — floor still **0.221.1**. +`felhom.eu/documentation/audits/DRILL-r329-r386-2026-08-23/evidence/` — 26 files: 5 red-proof +transcripts, 20 live-walk files, two full controller-log windows (1041 and 4044 lines) **pulled off +before each revert**, and the hub's own DB queries. -## 13. Register size +## 13. Teardown, three layers, and the end state + +1. **Guest 9201 / apps** — nothing provisioned. **All 17 app containers healthy.** `privatebin`'s + `app.yaml` restored from backup and the backup deleted; intent reads `running`. `demo-hp`'s + notification settings restored to `enabled_events: null`, no e-mail — their pre-drill state. No app + rebuilt, redeployed or restored; planted data untouched. +2. **Bake VM** — powered off, Gitea token and runner script **shredded**, `drill.qcow2` reverted to + `virgin`. Build guest 9100 exists only inside that reverted snapshot. No storage added anywhere, so + `pvesm status` has nothing to compare. +3. **Hub-side, stated explicitly.** The hub was **written** this session, unlike last: the deployment + is v0.107.0 via the manifest, and **two probe events remain as rows for `demo-hp`** from Scenario H + (`backup_failed`, "R-387 scenario H probe" and "…control"). They are inert records; named here + rather than left for someone to find. Nothing else: no appliance registered, no customer created, + no artifact manifest changed, floor untouched. + +**End state:** controller **0.223.0** and hub **0.107.0** deployed; golden **0.223.0** baked and +published but **NOT vouched**; floor still **0.222.0**; all apps running; planted data present. + +## 14. Register size | File | Before | After | |---|---|---| -| `OPEN-ITEMS.md` | 327,266 B | **328,325 B** | -| `CLOSED-ITEMS.md` | 68,464 B | **71,441 B** | +| `OPEN-ITEMS.md` | 328,325 B | **328,132 B** | +| `CLOSED-ITEMS.md` | 71,441 B | **74,642 B** | -R-383 and R-384 moved to CLOSED compressed; R-385 (closed) and R-386 (open) filed. OPEN grew by -~1 KB despite two closures because R-386 is a substantial new finding — recorded rather than -smoothed over. +R-329 and R-386 closed and compressed; **R-387** (closed) and **R-388** (the notification-model +product decision — open, operator's call) filed. -## 14. Observations — noticed, documented, NOT acted on +## 15. Observations — noticed, documented, NOT acted on -1. **R-329 is live and now matters much more.** `app_start_failed` is pushed with severity **`warn`**, - which is not in the hub's vocabulary (`{info, warning, error, critical}`) and coerces silently to - `info`, e-mailing nobody, while the POST still returns 200. Observed again today: - `PushEvent: type=app_start_failed severity=warn`. **R-384 makes this event actually fire, so a - known-broken severity moved from unreachable to load-bearing.** Not in scope; not touched. -2. **Two files carry pre-existing `gofmt` drift** — `internal/backup/offbox.go` and - `internal/backup/offbox_recovery_cli.go`. Confirmed pre-existing by stashing this session's work - and re-running `gofmt -l`. Not touched (§12 forbids nearby refactors). -3. **The runbook's golden-bake step is missing `pveam update`.** On the `virgin` snapshot the template - index is stale, so `pveam available` offers `13.1-2` and downloading it fails with - `400 Parameter verification failed. template: no such template`. Recorded in the bake evidence - README; the runbook itself was not edited. -4. **The register's own suggested fix for R-384 was wrong** — it proposed a sustained-`unhealthy` - threshold on the `crashLoopAfter` model. The defect needed no threshold at all, only an ordering. - Recorded in the CLOSED entry so the next reader sees that a register remedy is a hypothesis. -5. **Deliberately left open, untouched:** R-102, R-359, R-361's sibling surfaces. +1. **The operator cooldown key has no app identifier, and it now bites.** PrivateBin's alarm four + minutes after BookStack's was logged `suppressed — operator cooldown 1h, + key=demo-hp:app_start_failed`, so **only the first app-down per hour e-mails the operator**. This is + R-182's known cooldown-key shape; it was harmless while `app_start_failed` was undeliverable and is + not any more. **Same pattern as R-329 itself: a known-broken thing moved from unreachable to + load-bearing.** Not fixed here. +2. **The settings page grew 12 → 15 toggles in one session** — one new alarm, plus two compound + toggles split into four. Recorded as the argument inside R-388. +3. `internal/notify/notifier.go` carries **pre-existing** gofmt drift in an unrelated const block, + confirmed by stashing this session's work and re-running `gofmt -l`. Not touched (§12). +4. **The golden-bake runbook still lacks `pveam update`** — second consecutive bake to hit the stale + index on the `virgin` snapshot, presenting as `400 … no such template`. +5. **Deliberately left open, untouched:** R-102, R-359, R-385, and R-388's redesign. + +### CI runs, confirmed by ID + +| Commit | Repo | CI `id` | `run_number` | Result | +|---|---|---|---|---| +| `9832760` | felhom-controller | **408** | 88 | success | +| `68a9f54` | felhom.eu | **409** | 261 | success | +| `2f7c9a6` | felhom.eu | **410** | 262 | success | + +(The controller docs commit's run is confirmed after its push and is the next `id` in that repo.) diff --git a/controller/README.md b/controller/README.md index 2547ba5..7a24d99 100644 --- a/controller/README.md +++ b/controller/README.md @@ -1971,6 +1971,22 @@ The controller pushes structured events to the Hub's `/api/v1/event` endpoint. T **Core method:** `PushEvent(eventType, severity, message, details)` — non-blocking goroutine, 2 retries with 3s backoff, never blocks the caller. +> **⚠ THE SEVERITY VOCABULARY IS THE HUB'S, AND IT IS EXACT: `{info, warning, error, critical}`.** +> The hub **coerces anything else to `info` at ingest**, and `info` is dropped by `severityNotifies` +> **before both** delivery legs. So a severity outside that set means the event is stored, the POST +> returns `200`, the dashboard shows it — and **it is e-mailed to nobody**. +> +> This shipped twice: `DiskAlertKind.Severity` sent `warn` until v0.215.0, and `app_start_failed` sent +> it until **v0.223.0** — **91 of those events were stored and not one was ever delivered.** It is now +> pinned by an AST walk over the whole controller +> (`TestR329_EveryEmittedSeverityIsInTheHubVocabulary`); the six call sites that pass a *variable* are +> registered by name, so a new one fails the test. Since v0.107.0 the hub also logs a `WARN` naming +> any severity it had to rewrite. Full contract: `felhom.eu/documentation/architecture/08-alarm-ladder.md` §6.1. +> +> **Who receives what.** `processOperator` consults only the operator switch, the address and a +> one-hour cooldown — **never customer preferences** — so a valid severity always reaches the operator. +> The customer leg additionally consults `operatorOnlyEvents` and the customer's own enabled events. + #### Event Types | Event Type | Severity | Trigger | @@ -1986,14 +2002,14 @@ The controller pushes structured events to the Hub's `/api/v1/event` endpoint. T | `health_critical` | error | Health status critical (any→fail) | | `health_recovered` | info | Health status recovers (fail/warn→ok) | | `disk_warning` | warning | Disk usage crosses 90% | -| `disk_critical` | error | Disk usage crosses 95% | +| `disk_critical` | **critical** | Disk usage crosses 95% (this row read `error` until v0.223.0; the emitter is `fillwatch.Band.Severity()` and it has always sent `critical`) | | `storage_disconnected` | error | Storage drive physically removed | | `storage_reconnected` | info | Storage drive reconnected | | `controller_started` | info | Controller process starts | | `controller_updated` | info/error | Self-update success or failure | | `app_deployed` | info | New app deployed via API | | `app_removed` | info | App removed via API | -| `app_start_failed` | warn | A DEPLOYED app is not running (fix-3) — fired ONCE per running→down transition | +| `app_start_failed` | **warning** | A DEPLOYED app is not running (fix-3) — fired ONCE per running→down transition. **Customer-switchable („Alkalmazás nem fut"), OFF by default; the OPERATOR is e-mailed regardless.** Was `warn` until v0.223.0 — see the severity note below | | `disaster_recovery_started` | warning | DR restore begins | | `disaster_recovery_completed` | info/error | DR restore finishes (success/partial) |