diff --git a/REPORT.md b/REPORT.md index e31fb26c..f673939f 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,186 +1,104 @@ -# REPORT — five process-domain skills + check_skills.py (2026-08-25) +# REPORT — R-331: the operator Backup card said every customer had no backups -Documentation and agent-configuration only. No Go code written or changed, nothing built, nothing -deployed. Only DooPlex was touched, and on it only this repo's working tree and `~/.claude/skills/`. +**Hub v0.109.0 (with controller v0.225.0) · 2026-08-30** -## 1. Confirmed baseline +--- -`felhom.eu` @ `ebdc04601d37db4c732245a6a9df777bd28fe6df` (2026-08-23T14:12:23+02:00). Clean tree, -`HEAD == origin/main`, verified before any edit. Matches the baseline in the task file. +## 1. What was wrong -## 2. Files created / modified - -Created: -- `/mnt/5_hdd/felhom.eu/git/felhom.eu/skills/felhom-evidence/SKILL.md` -- `/mnt/5_hdd/felhom.eu/git/felhom.eu/skills/felhom-diagnosis/SKILL.md` -- `/mnt/5_hdd/felhom.eu/git/felhom.eu/skills/felhom-plain-language/SKILL.md` -- `/mnt/5_hdd/felhom.eu/git/felhom.eu/skills/felhom-handoff/SKILL.md` -- `/mnt/5_hdd/felhom.eu/git/felhom.eu/skills/felhom-doc-authoring/SKILL.md` -- `/mnt/5_hdd/felhom.eu/git/felhom.eu/skills/SOURCES.md` -- `/mnt/5_hdd/felhom.eu/git/felhom.eu/scripts/check_skills.py` - -Modified: -- `/mnt/5_hdd/felhom.eu/git/felhom.eu/documentation/runbooks/workspace-CLAUDE.md` (one line: stale - count "the four Felhom skills" → "the Felhom skills"; validator named) -- `/mnt/5_hdd/felhom.eu/git/felhom.eu/documentation/backlog/OPEN-ITEMS.md` (three rows) -- `/mnt/5_hdd/felhom.eu/git/felhom.eu/scripts/CHANGELOG.md` -- `/mnt/5_hdd/felhom.eu/git/felhom.eu/CONTEXT.md` -- `/mnt/5_hdd/felhom.eu/git/felhom.eu/REPORT.md` (this file) - -**DEVIATION FROM THE TASK FILE §6.3.** It asked for an entry in `felhom.eu/CHANGELOG.md`. **That file -does not exist** — this repo keeps per-area changelogs (`scripts/`, `website/`, `hub/`), as -`CONTEXT.md`'s own header states. The entry went to `scripts/CHANGELOG.md`, the closest owning area, -since the checker lives there. `skills/` has no changelog of its own and none was created. - -## 3. Commit hashes pushed to `main` - -`c30430c530a2fcf3b0aca03c6ef5d266e30b8314` — *skills: five process-domain skills + check_skills.py* - -Pushed with **`git push --no-verify`**; the reason is stated in §12 item 6, as the hook requires. -Baseline `ebdc046` → `c30430c`, one commit, no branch. - -## 4. Check-script results and the red-proof - -`python3 scripts/check_skills.py` → **exit 0**, nine skills: +The hub customer page's **Backup** card read, for **every customer, indefinitely**: ``` -OK felhom-app-catalog 126 lines live-linked -OK felhom-build-deploy 179 lines live-linked -OK felhom-diagnosis 90 lines live-linked -OK felhom-doc-authoring 86 lines live-linked -OK felhom-evidence 90 lines live-linked -OK felhom-handoff 69 lines live-linked -OK felhom-plain-language 55 lines live-linked -OK felhom-testing 99 lines live-linked -OK felhom-ui-design 71 lines live-linked -WARN felhom-build-deploy: 179 lines, OVER the 150-line limit — grandfathered (R-394 — 179 lines - when the limit was introduced; trim is a scoped session) - -PASS: 9 skill(s) well formed. +Enabled Yes Snapshots 0 +Repo Size 0 MB Integrity Unknown ``` -**RED-PROOF — RUN, AND SEEN FAILING.** It did not pass without having been seen to fail. +Measured on `demo-hp` 2026-08-30, at which moment the truth was: -1. `grep -v '^description:'` removed the `description` line from `skills/felhom-evidence/SKILL.md`. -2. Re-ran → **exit 1**. Exact failure message seen: - `- /mnt/5_hdd/felhom.eu/git/felhom.eu/skills/felhom-evidence/SKILL.md: frontmatter field 'description' is missing or empty` -3. Restored from a scratchpad copy → exit 0; `git status --porcelain` showed no modified tracked - file, only the intended new untracked paths. - -This is a **positive control**: the checker was shown finding a planted fault before its silence on -the other eight was read as "all well formed". - -## 5. Install output, both runs, and the samefile confirmation - -Run 1 — five newly linked, four already installed, no FAIL, no copy-mode note, **exit 0**: -``` -OK felhom-app-catalog symlink (already installed, live-linked to repo) -OK felhom-build-deploy symlink (already installed, live-linked to repo) -OK felhom-diagnosis symlink -> /mnt/5_hdd/felhom.eu/git/felhom.eu/skills/felhom-diagnosis -OK felhom-doc-authoring symlink -> /mnt/5_hdd/felhom.eu/git/felhom.eu/skills/felhom-doc-authoring -OK felhom-evidence symlink -> /mnt/5_hdd/felhom.eu/git/felhom.eu/skills/felhom-evidence -OK felhom-handoff symlink -> /mnt/5_hdd/felhom.eu/git/felhom.eu/skills/felhom-handoff -OK felhom-plain-language symlink -> /mnt/5_hdd/felhom.eu/git/felhom.eu/skills/felhom-plain-language -OK felhom-testing symlink (already installed, live-linked to repo) -OK felhom-ui-design symlink (already installed, live-linked to repo) -``` -Run 2 — all nine `(already installed, live-linked to repo)`, **exit 0**. Idempotent. - -`os.path.samefile` between `~/.claude/skills//SKILL.md` and the repo file, proving a symlink -and not a copy: -``` -felhom-diagnosis samefile=True islink(dir)=True -felhom-doc-authoring samefile=True islink(dir)=True -felhom-evidence samefile=True islink(dir)=True -felhom-handoff samefile=True islink(dir)=True -felhom-plain-language samefile=True islink(dir)=True -``` - -## 6. Line count of each new SKILL.md (limit: under 150) - -| Skill | Lines | +| source | value | |---|---| -| `felhom-evidence` | 90 | -| `felhom-diagnosis` | 90 | -| `felhom-doc-authoring` | 86 | -| `felhom-handoff` | 69 | -| `felhom-plain-language` | 55 | +| the box's own `settings.json` | `snapshot_count: 67, repo_size_bytes: 140829678, stats_known: true` | +| that night's controller log | `[offbox] backup OK: 8 app(s) backed up, 67 snapshot(s), 2m14s` | +| **this hub's own Offsite page** | `0.1 GB` used of a `50 GB` quota — read from the same stored report | -All five under the limit. None had to be split. +**A card that reads "no backups" over a working backup is worse than no card.** It is the R-88 +direction of failure — degrading to *no backup* rather than to *unknown* — on the one screen an +operator consults to answer "is this customer protected?". -## 7. NOT YET VALIDATED — awaiting the operator +## 2. Root cause -**Whether each skill FIRES is behavioural and was NOT proven here.** The checker proves a skill is -loadable; it cannot prove the model reaches for it. That is decided by the `description` wording and -no mechanical check settles it. +The card rendered the report's **`backup`** object. Its `snapshot_count`, `repo_size_mb` and +`integrity_ok` fields have had **no producer** since disk-tier restic moved to the host agent (slice +8C) — the controller's `buildBackupReport` leaves them zero *deliberately* and says so in a comment. +The zeros were correct values for dead fields, rendered as if live. -One observable WAS obtained and is stated for what it is, not more: after installation all five -appeared in this session's own skill listing with their descriptions intact. That proves the harness -discovered and parsed them. **It does not prove any of them fires on a real prompt.** +**The data was never missing.** The live numbers ride in the report's **`offsite`** object, which this +package **already** reads for the Offsite page (`offsiteUsageBytes`) and which `monitor.OffsiteChecker` +**already** drives fill and staleness alarms from. That the Offsite page rendered demo-hp's real usage +from the same stored report, at the same moment the Backup card said `0 MB`, is the proof the bytes +were arriving. This is a **render fix over an existing feed**, not a new pipeline. -The operator's check, in a **fresh** Claude Code session (one probe phrase each): +## 3. Why it was not a one-line template swap -| Skill | Probe phrase to type | Pass looks like | -|---|---|---| -| `felhom-evidence` | *"Are we sure the stopped stack is intentional?"* | the reply grades the claim into one of the five tiers; **fails** if it answers ungraded, or uses "clearly" / "obviously" | -| `felhom-diagnosis` | *"The controller stopped working after a restart, why?"* | it asks for or builds a failing command FIRST and refuses to theorise; **fails** if it starts reading code and proposing causes | -| `felhom-plain-language` | *"wait, what"* after any technical answer | it rewrites the whole answer shorter; **fails** if it defends the original or asks which part was unclear | -| `felhom-handoff` | *"I need to stop, pick this up later"* | it writes a note to `/tmp/felhom-handoff-.md`; **fails** if the plan is only in the conversation | -| `felhom-doc-authoring` | *"write a skill for X"* | it treats the `description` as the pointer that decides reachability, and holds under 150 lines; **fails** if it writes the body first and the frontmatter as an afterthought | +`snapshot_count: 0` means two opposite things — *this repository holds nothing* and *nobody has ever +measured this repository*. **R-225 measured that confusion one layer down**: a rebuilt box rendered +„Tarolo meret · 0 pillanatkep" over a store that really held snapshot `f3d9cd67`, and the controller's +`StatsKnown` fixed it there. It was never on the wire, so rendering the count without it would have +**moved R-225 up to the hub instead of fixing anything**. Controller v0.225.0 now forwards +`stats_known`. -## 8. Evidence +## 4. What changed -N/A. No phase ran on any machine other than DooPlex, nothing was reverted, and no snapshot was -restored. The one temporary mutation (the red-proof) was restored in the same step that made it, and -its output is quoted in §4 rather than left on disk. +`hub/internal/web/backup_card.go` builds a typed `backupCardView` — resolved in Go, because the card's +whole subject is a distinction a template `{{if}}` chain over `map[string]interface{}` float64s cannot +keep: -## 9. Teardown - -N/A — this run provisioned nothing. No guest, no VM, no rig, no scratch host. - -## 10. Register - -| Row | Title | +| report state | card shows | |---|---| -| **R-392** | No architecture document covers the two-AI workflow | -| **R-393** | Decision-log skill for unattended runs — deferred, with the reason | -| **R-394** | `felhom-build-deploy/SKILL.md` is 179 lines, over the limit its own repo now enforces | +| no `offsite` object at all | "No off-site data reported" — **and says explicitly this is not the same as "no backups"** | +| `enabled:false` + declared `state` | the blocker by name (`needs_credential`) — a different operator action from "not enabled" | +| enabled, `stats_known:false` | **—**, plus "never been measured". Never `0` | +| enabled, `stats_known:true` | the real count and size, **including a real `0`** — measured empty is knowledge | -`documentation/backlog/OPEN-ITEMS.md` — **335,212 bytes** after the three rows (331,024 before). -No rows were closed this session, so nothing was compressed or rehomed. +**A pre-v0.225.0 controller sends no `stats_known`, which unmarshals to false → "unknown".** That is +the fail-safe direction: upgrading the hub ahead of the fleet must not tell the operator that every +un-upgraded customer has zero backups. Pinned by a test. -## 11. Capability map +**The Integrity row is deleted, not re-sourced.** Nothing produces it: the controller runs no integrity +check, and `NotifyIntegrityOK` / `NotifyIntegrityFailed` exist and are **called from nowhere**. A row +that can only ever read "Unknown" is not information, and one that could read "OK" from an unwritten +field would be a lie. -**No product capability changed and no row in `documentation/architecture/00-capability-map.md` was -touched.** This task changes no product behaviour: no binary, no endpoint, no template, no guest, no -host. `documentation/backlog/ROADMAP.md` is likewise unchanged — this is not product work. +`fmtBytesAuto` is new rather than reusing `fmtBytesGB`: that one is fixed at GB because it renders +against GB quotas, and it turns demo-hp's real 140 829 678 bytes into `0.1 GB` — which on a card whose +entire defect was under-reporting a real backup reads as "nearly nothing". -## 12. Observations +## 5. Tests and the red-proof -1. **`felhom-build-deploy/SKILL.md` is 179 lines, over the 150-line limit this task introduced.** - Found by the new checker on its first run. Not edited — the task scoped the four pre-existing - skills out — and made a named single-entry `GRANDFATHERED` exception printed as a WARN on every - run so it cannot fade. **FILED: R-394** -2. **The task file specified a check on every `skills/*/SKILL.md` that its own "do not edit the - existing skills" rule made unsatisfiable.** The two constraints met on `felhom-build-deploy`. - Resolved by naming the exception rather than weakening the rule or editing the file; both - alternatives would have hidden a real finding. **FILED: R-394** (same row — it is the same fact, - and a second row would be the duplicate the one-register ruling exists to prevent). -3. **No architecture document covers the agent-tooling layer.** Found by trying to fill the task - template's owning-document field and being unable to. **FILED: R-392** -4. **A sixth skill — a decision log for unattended runs — was considered and held back**, because it - needs a helper script and a storage convention rather than a text file. **FILED: R-393** -5. **`felhom.eu` has no root `CHANGELOG.md`**, though the task file and the workspace `CLAUDE.md` - both refer to one for this repo. This repo deliberately keeps per-area changelogs, which - `CONTEXT.md`'s header states. **NOT-A-FINDING: the per-area convention is deliberate and - documented in `CONTEXT.md`; the entry went to `scripts/CHANGELOG.md` and the deviation is stated - in §2. Nothing is missing — only the task file's assumption was wrong.** -6. **The push used `git push --no-verify`, and this is the required statement of that.** The - pre-push hook's `--fast` gate run convicted on **due-checks**, not on anything this session - changed: **R-341's dated check came due 2026-08-25**, the day of this session. The DUE-CHECKS - block was not touched here (`git diff` on it is empty), and the other eleven gates — including - `observations`, `one-register` and `instructions` — all passed. Taking R-341's measurement is a - live-host systemd uptime reading, a different task with its own preconditions, and moving its date - to clear the gate would have silently deferred someone else's check to make this push convenient. - **FILED: R-341** — the row already exists and is the correct home; a new row would be the - duplicate the one-register ruling exists to prevent. +`r331_backup_card_test.go` asserts the **rendered page**, using demo-hp's real reported values, so a +regression fails against the same numbers the defect was measured against. **The defect lived in the +template's choice of source object, so a test one layer below it would have been green against the +shipped bug** — which is why these drive `handleCustomerUnified` and grep the HTML. + +**RED-PROOF (run 2026-08-30):** restoring the pre-fix card markup fails all four tests — +`the rendered Backup card does not contain demo-hp's real snapshot count (67)`, +`the card does not carry the real repository size (134.3 MB ...)`, +`the card still shows an Integrity row`, plus every branch of the three-way ruling. Restored +immediately; `git diff` clean. + +**Green gate:** `go build ./... && go vet ./... && go test ./...` in `hub/` — 18 packages, rc 0. + +## 6. Deployment + +PENDING at the time of writing — see the follow-up commit. The two halves ship independently and in +either order: the hub renders "unknown" for any box still on controller 0.224.0, which is correct +rather than wrong. + +## 7. Not done, and why + +- **No staleness verdict on the card.** `monitor.OffsiteChecker` already owns that and alarms on it. A + second verdict over the same data is two things that can disagree — a shape this codebase has already + paid for (`LastRun` vs `LastSuccess`, R-100). +- **The dead `backup` fields were not removed from the controller's wire format.** Removing them would + stop historical reports already in this hub's store from parsing, for no gain — nothing renders them + now, and a controller-side test fails if anything starts producing them. diff --git a/REUSE.md b/REUSE.md index 07fb4729..b4c93089 100644 --- a/REUSE.md +++ b/REUSE.md @@ -65,6 +65,7 @@ | `(*Server).hostStatus` + `hostStatusClass`/`hostStatusLabel` | hub/internal/web/hosts.go (~L16/34/48) | `(lastReport *time.Time) string` | Host liveness badge | Uses the SAME threshold as HostStalenessChecker (down = 2× stale) — never invent a second definition. | | `parseSQLiteTime` | hub/internal/store/store.go (~L1160) | `(s string) time.Time` | Parsing ANY timestamp read from SQLite | modernc/sqlite returns multiple formats; raw `time.Parse` will intermittently zero out. Always use this. | | `compareVersions` | hub/internal/web/server.go (~L571) | `(a, b string) int` | X.Y.Z comparisons in web (floor checks, update-available) | Returns 0 on parse error — unparseable compares as "equal" (see §3). | +| `reportBackupCard` + `backupCardView` + `fmtBytesAuto` (R-331, v0.109.0) | hub/internal/web/backup_card.go | `(reportJSON string) backupCardView` / `(int64) string` | THE customer-page Backup card — everything it shows about a customer's backups | **Reads the report's `offsite` object, NEVER `backup`.** The `backup` object's `snapshot_count`/`repo_size_mb`/`integrity_ok` have had no producer since slice 8C and rendering them told every operator every customer had `Snapshots 0` (measured on demo-hp over a repo holding 67). **Always consult `StatsKnown` before believing a zero** — absent/false means "never measured", NOT "empty", and those are opposite news (R-225 measured the same confusion one layer down). Resolved in Go, not the template, because a `{{if}}` chain over `Report`'s `map[string]interface{}` float64s cannot keep the absent/zero distinction the card is entirely about. `fmtBytesAuto` scales MB/GB/TB — do NOT swap in `fmtBytesGB`, which renders demo-hp's real 140 829 678 B as `0.1 GB`. | ### Host views & lifecycle / offsite endpoints (v0.47.0, hub/internal/web + store) @@ -174,6 +175,7 @@ | Inline `stringData` secrets à la manifests/felhom.secret.yaml | Commits real credentials to git (healthchecks superuser pw, umami APP_SECRET/POSTGRES_PASSWORD, gitea-creds admin password still live there). | Out-of-band `kubectl create secret` + `secretKeyRef` (hub.yaml resend-api pattern; runbook documentation/runbooks/secrets.md) | | `kubectl apply` / `kubectl set image` on manifests/ | ArgoCD app `felhom` reverts drift on next sync; live state lies about git. | Edit manifest in git → push → ArgoCD sync (CLAUDE.md steps 3–5) | | `:latest` image tag in manifests | Re-push doesn't change the manifest → no redeploy; Synced/Rollback misreport. | Pinned version tag, bumped per deploy | +| The report's `backup` object for snapshots / repo size / integrity (`snapshot_count`, `repo_size_mb`, `integrity_ok`) | **No producer since slice 8C** — the controller's `buildBackupReport` leaves all four zero deliberately and says so. Rendering them gave every customer `Snapshots 0 · Repo Size 0 MB · Integrity Unknown` indefinitely (R-331, measured on demo-hp over a repository holding 67 snapshots). `integrity_ok` is worse than stale: the controller runs no integrity check at all, so it can only ever be "Unknown" or a lie. | The report's `offsite` object (`backup.OffboxReportStatus`) via `reportBackupCard` — and check `stats_known` before trusting a zero | | grep/regex hunting emoji in website HTML | Windows grep false-negatives multibyte emoji (proven in D0). | `python scripts/site_gates.py` (codepoint-range check) | | Adding a website page without touching site_gates.py | `PAGES` list (scripts/site_gates.py ~L22) is explicit — an unlisted page is silently ungated (BOM/nav/emoji drift undetected). | Add the filename to `PAGES` in the same commit | diff --git a/hub/CHANGELOG.md b/hub/CHANGELOG.md index 8d8cac41..3fe237dc 100644 --- a/hub/CHANGELOG.md +++ b/hub/CHANGELOG.md @@ -1,3 +1,67 @@ +## v0.109.0 — the Backup card told every operator that every customer had no backups (2026-08-30, R-331) + +### R-331 — `Snapshots 0` over a repository holding 67 + +The customer page's **Backup** card read `Snapshots 0 · Repo Size 0 MB · Integrity Unknown` for +**every customer, indefinitely**. Measured on `demo-hp` 2026-08-30 while that same night's controller +log said `[offbox] backup OK: 8 app(s) backed up, 67 snapshot(s), 2m14s` and the box's own settings +held `snapshot_count: 67, repo_size_bytes: 140829678, stats_known: true`. + +**A card that reads "no backups" over a working backup is worse than no card** — it is the R-88 +direction of failure (degrading to *no backup* rather than to *unknown*) on the one screen an operator +consults to answer "is this customer protected?". + +**The data was never missing.** The card rendered the report's `backup` object, whose +`snapshot_count` / `repo_size_mb` / `integrity_ok` fields have had **no producer** since disk-tier +restic moved to the host agent (slice 8C) — the controller's `buildBackupReport` says so in a comment +and leaves them zero. The live numbers were in the `offsite` object all along, which **this package +already reads** (`offsiteUsageBytes`, for the Offsite page) and which `monitor.OffsiteChecker` already +drives fill and staleness alarms from. Proof the bytes were arriving: the Offsite page rendered +demo-hp's usage as `0.1 GB` from that very object while the Backup card said `0 MB`. + +So this is a **render fix over an existing feed**, not a new pipeline. + +### Why it was not a one-line template swap + +`snapshot_count: 0` means two opposite things — *this repository holds nothing* and *nobody has ever +measured this repository*. **R-225 already measured that confusion one layer down**: a rebuilt box +rendered „Tarolo meret · 0 pillanatkep" over a store that really held snapshot `f3d9cd67`, and +`StatsKnown` is what fixed it in the controller's own UI. Rendering the count without consulting it +would have moved R-225 up to the hub instead of fixing anything. Controller **v0.225.0** now forwards +`stats_known`; the card is a **three-way** ruling, resolved in Go (`backup_card.go`) rather than in a +template `{{if}}` chain over `map[string]interface{}` float64s: + +| report state | card shows | +|---|---| +| no `offsite` object at all | "No off-site data reported" — **and says this is not the same as "no backups"** | +| `enabled:false` + declared `state` | the blocker by name (`needs_credential`) — a different operator action from "not enabled" | +| enabled, `stats_known:false` | **—**, plus "never been measured". Never `0` | +| enabled, `stats_known:true` | the real count and size, **including a real `0`** — measured empty is knowledge | + +**A pre-v0.225.0 controller sends no `stats_known`, which unmarshals to false → "unknown".** That is +the fail-safe direction and it is pinned by a test: upgrading the hub ahead of the fleet must not tell +the operator that every un-upgraded customer has zero backups. + +### The Integrity row is DELETED, not re-sourced + +Nothing in the controller produces it. There is no integrity check; `NotifyIntegrityOK` and +`NotifyIntegrityFailed` exist and are **called from nowhere**. A row that can only ever read "Unknown" +is not information, and one that could read "OK" from an unwritten field would be a lie. + +### Tests + +`r331_backup_card_test.go` asserts the **rendered page**, using demo-hp's real reported values, so a +regression fails against the same numbers the defect was measured against. The defect lived in the +template's choice of source object, so a test one layer below it would have been green against the +shipped bug. + +**RED-PROOF:** restoring the pre-fix card markup fails all four tests, reporting `67` and `134.3 MB` +absent from the page and the `Integrity` row present. + +`fmtBytesAuto` is new rather than reusing the Offsite page's `fmtBytesGB`: that one is fixed at GB +because it renders against GB quotas, and it turns demo-hp's real 140 829 678 bytes into `0.1 GB` — +which on a card whose entire defect was under-reporting a real backup reads as "nearly nothing". + ## v0.108.0 — only the first broken app per hour reached the operator (2026-08-23, R-389) **The hour is unchanged. The GRAIN is what was wrong.** diff --git a/hub/internal/web/backup_card.go b/hub/internal/web/backup_card.go new file mode 100644 index 00000000..430b083b --- /dev/null +++ b/hub/internal/web/backup_card.go @@ -0,0 +1,148 @@ +package web + +import ( + "encoding/json" + "fmt" + "time" +) + +// ── R-331 — the operator Backup card told every customer they had no backups ───────────────────── +// +// THE DEFECT. `customer_unified.html`'s Backup card rendered `snapshot_count`, `repo_size_mb` and +// `integrity_ok` out of the report's `backup` object. Those three fields have had NO PRODUCER since +// disk-tier restic moved to the host agent (slice 8C): the controller's `buildBackupReport` says so +// in a comment and leaves them zero, so the card read `Snapshots 0 · Repo Size 0 MB · Integrity +// Unknown` for every customer, forever. Measured on `demo-hp` 2026-08-30, while the box's own log for +// that night said `[offbox] backup OK: 8 app(s) backed up, 67 snapshot(s), 2m14s`. +// +// **A card that reads "0 snapshots" over a working backup is worse than no card.** It is the R-88 +// direction of failure — degrading to "no backup" rather than to "unknown" — on the one screen an +// operator would consult to answer "is this customer protected?". +// +// THE DATA WAS NEVER MISSING. The live numbers ride in the report's `offsite` object, which this +// package already reads for the Offsite page (`offsiteUsageBytes`) and which `monitor.OffsiteChecker` +// already drives fill and staleness alarms from. The card was simply pointed at the wrong object. +// So this is a RENDER fix over an existing feed, not a new pipeline — and `integrity_ok` is dropped +// rather than re-sourced, because nothing in the controller has produced it since 8C either: +// `NotifyIntegrityOK` / `NotifyIntegrityFailed` exist and are called from nowhere. +// +// WHAT MADE IT MORE THAN A ONE-LINE TEMPLATE SWAP. `snapshot_count: 0` means two opposite things — +// "this repository holds nothing" and "nobody has ever measured this repository" — and the report +// could not tell them apart until controller v0.225.0 started forwarding `stats_known`. R-225 +// measured that exact confusion one layer down: a rebuilt box rendered „Tároló méret · 0 pillanatkép" +// over a store that really held snapshot `f3d9cd67`. Rendering the count without consulting +// `stats_known` would have moved R-225 from the controller UI to the hub UI instead of fixing it. + +// backupCardView is what the Backup card renders. Every field is resolved in Go rather than in the +// template, so the three-way "no data / unknown / known" ruling is unit-testable and lives in one +// place — a template `{{if}}` chain over a `map[string]interface{}` of float64s is neither. +type backupCardView struct { + // --- local app-data leg (the report's `backup` object) --- + // These two are the ONLY fields of that object with a live producer, which is why nothing else + // from it appears on the card any more. + LocalEnabled bool + LastDBDump string // humanised; "—" when the box has never dumped + + // --- off-site leg (the report's `offsite` object) --- + + // OffsiteReported is false when the report carried no `offsite` object at all: a pre-v0.109.0 + // controller, or a box that has never had the tier. The card then says so and shows NO numbers — + // the same rule `offsiteBoxTile` already follows ("absent, not a zeroed tile"). + OffsiteReported bool + OffsiteEnabled bool + // OffsiteState carries a DECLARED state (`needs_credential`, `awaiting_recovery_key`) for a box + // that reports an object while disabled. Rendering it is the difference between "no off-site + // backup" and "off-site backup is blocked waiting for a key" — an operator action item. + OffsiteState string + + // StatsKnown is false both when the repository was never measured AND when the controller is too + // old to say. Both are ignorance, and the card must show ignorance, never zero. + StatsKnown bool + Snapshots int + RepoSize string // humanised; only meaningful when StatsKnown + + LastSuccess string // humanised age of the last run that actually SUCCEEDED; "—" when never + LastStatus string // "ok" | "incomplete" | "error" | "running" + QuotaStr string // "" when the target has no soft quota (dedicated boxes) +} + +// reportBackupCard parses one customer's stored report into the card view. A report that will not +// parse yields a zero view, which renders as "nothing reported" — never as zeroed numbers. +// +// It takes the raw JSON rather than the already-parsed `map[string]interface{}` the page also holds, +// for the reason the sibling readers in this file's package do: typed decoding keeps the int/float64 +// and absent/zero distinctions that a generic map destroys, and those distinctions are the whole +// subject of this card. +func reportBackupCard(reportJSON string) backupCardView { + var r struct { + Backup *struct { + Enabled bool `json:"enabled"` + LastDBDump *time.Time `json:"last_db_dump"` + } `json:"backup"` + Offsite *struct { + Enabled bool `json:"enabled"` + State string `json:"state"` + LastStatus string `json:"last_status"` + LastSuccess string `json:"last_success"` + SnapshotCount int `json:"snapshot_count"` + RepoSizeBytes int64 `json:"repo_size_bytes"` + QuotaGB int `json:"quota_gb"` + // StatsKnown is ABSENT on a controller below v0.225.0, and absent unmarshals to false — + // which is the correct fail-safe direction: a box that cannot answer is unknown, never + // empty. See OffboxReportStatus.StatsKnown in the controller. + StatsKnown bool `json:"stats_known"` + } `json:"offsite"` + } + var v backupCardView + if json.Unmarshal([]byte(reportJSON), &r) != nil { + return v + } + + if r.Backup != nil { + v.LocalEnabled = r.Backup.Enabled + v.LastDBDump = "—" + if r.Backup.LastDBDump != nil && !r.Backup.LastDBDump.IsZero() { + v.LastDBDump = timeAgo(*r.Backup.LastDBDump) + } + } + + if r.Offsite == nil { + return v + } + v.OffsiteReported = true + v.OffsiteEnabled = r.Offsite.Enabled + v.OffsiteState = r.Offsite.State + v.LastStatus = r.Offsite.LastStatus + v.StatsKnown = r.Offsite.StatsKnown + if v.StatsKnown { + v.Snapshots = r.Offsite.SnapshotCount + v.RepoSize = fmtBytesAuto(r.Offsite.RepoSizeBytes) + } + v.LastSuccess = "—" + if t, err := time.Parse(time.RFC3339, r.Offsite.LastSuccess); err == nil { + v.LastSuccess = timeAgo(t) + } + if r.Offsite.QuotaGB > 0 { + v.QuotaStr = fmt.Sprintf("%d GB", r.Offsite.QuotaGB) + } + return v +} + +// fmtBytesAuto scales to the unit that keeps a repository size readable. The Offsite page's +// fmtBytesGB is deliberately NOT reused here: it is fixed at GB because it renders against GB quotas +// where a common unit is the point, and it turns demo-hp's real 140 829 678 bytes into "0.1 GB" — +// which on a card whose entire defect was under-reporting a real backup reads as "nearly nothing". +func fmtBytesAuto(b int64) string { + switch { + case b >= 1<<40: + return fmt.Sprintf("%.2f TB", float64(b)/float64(int64(1)<<40)) + case b >= 1<<30: + return fmt.Sprintf("%.1f GB", float64(b)/float64(int64(1)<<30)) + case b >= 1<<20: + return fmt.Sprintf("%.1f MB", float64(b)/float64(int64(1)<<20)) + case b >= 1<<10: + return fmt.Sprintf("%.1f KB", float64(b)/float64(int64(1)<<10)) + default: + return fmt.Sprintf("%d B", b) + } +} diff --git a/hub/internal/web/configs.go b/hub/internal/web/configs.go index 92f77b07..3b7f1632 100644 --- a/hub/internal/web/configs.go +++ b/hub/internal/web/configs.go @@ -294,6 +294,10 @@ func (s *Server) handleCustomerUnified(w http.ResponseWriter, r *http.Request, c HasReports bool Customer *store.CustomerSummary Report map[string]interface{} + // BackupCard (R-331) is resolved in Go, not in the template: the card must distinguish + // "no data reported" from "measured empty" from "never measured", and a {{if}} chain over + // Report's float64s cannot. See backup_card.go. + BackupCard backupCardView OverallStatus string HostCause string // v0.53.0 roll-up: "" or "host down|stale|pending: " @@ -428,6 +432,15 @@ func (s *Server) handleCustomerUnified(w http.ResponseWriter, r *http.Request, c } offsiteUnprovisioned := offsiteView.Offsite.Enabled && offsiteView.Offsite.Type == "" + // R-331: the Backup card reads the report's `offsite` object, not the long-dead `backup` + // snapshot/size/integrity fields. Built here so the page holds one resolved view rather than + // re-deriving it in template conditionals. A nil customer (never reported) yields the zero view, + // which renders as "nothing reported". + var backupCard backupCardView + if customer != nil { + backupCard = reportBackupCard(customer.ReportJSON) + } + data := pageData{ CustomerID: customerID, CustomerName: name, @@ -443,6 +456,7 @@ func (s *Server) handleCustomerUnified(w http.ResponseWriter, r *http.Request, c HasReports: customer != nil, Customer: customer, Report: report, + BackupCard: backupCard, OverallStatus: overallStatus, HostCause: hostCause, diff --git a/hub/internal/web/r331_backup_card_test.go b/hub/internal/web/r331_backup_card_test.go new file mode 100644 index 00000000..d4c80220 --- /dev/null +++ b/hub/internal/web/r331_backup_card_test.go @@ -0,0 +1,162 @@ +package web + +import ( + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-hub/internal/store" +) + +// ── R-331 — the Backup card said "Snapshots 0" over a working backup ───────────────────────────── +// +// Measured on `demo-hp` 2026-08-30: the card read `Snapshots 0 · Repo Size 0 MB · Integrity Unknown` +// while the box's own settings held `snapshot_count: 67, repo_size_bytes: 140829678, stats_known: +// true` and that night's log said `[offbox] backup OK: 8 app(s) backed up, 67 snapshot(s), 2m14s`. +// The card rendered the report's `backup` object, whose snapshot/size/integrity fields have had no +// producer since disk-tier restic moved to the host agent; the live numbers were in the `offsite` +// object all along. +// +// These tests use demo-hp's REAL reported values, so a regression fails against the same numbers the +// defect was measured against rather than against invented ones. + +// demoHPReportJSON is the shape demo-hp actually sends (controller 0.225.0), trimmed to the objects +// the Backup card reads. +const demoHPReportJSON = `{ + "backup": {"enabled": true, "last_db_dump": "2026-08-30T02:15:01Z"}, + "offsite": {"enabled": true, "escrow_state": "escrowed", "last_status": "ok", + "last_success": "2026-08-30T02:17:19Z", "snapshot_count": 67, + "repo_size_bytes": 140829678, "quota_gb": 50, "stats_known": true} +}` + +func renderWithReport(t *testing.T, reportJSON string) string { + t.Helper() + s, st := newTestServer(t) + if err := st.SaveCustomerConfig(&store.CustomerConfig{ + CustomerID: "demo-hp", CustomerName: "Demo HP", Domain: "enkisfelhom.hu", + RetrievalPassword: "pw", APIKey: "k", Status: "active", + }); err != nil { + t.Fatal(err) + } + if err := st.SaveReport("demo-hp", []byte(reportJSON)); err != nil { + t.Fatal(err) + } + return renderCustomerPage(t, s, "demo-hp") +} + +// THE CONSEQUENCE TEST. Not "does reportBackupCard return 67" — does the page an operator actually +// looks at contain 67. The defect lived in the template's choice of source object, so a test one +// layer below it would have been green against the shipped bug. +// +// RED-PROOF (run 2026-08-30, recorded in REPORT.md): point the card back at `.Report.backup` → +// this fails with the rendered page carrying `Snapshots 0` and no `67`. +func TestBackupCard_RendersTheRealSnapshotCount(t *testing.T) { + html := renderWithReport(t, demoHPReportJSON) + + if !strings.Contains(html, ">67<") { + t.Error("the rendered Backup card does not contain demo-hp's real snapshot count (67) — " + + "this is the defect: an operator reading this page concludes the customer has no backups") + } + if !strings.Contains(html, "134.3 MB") { + t.Error("the card does not carry the real repository size (134.3 MB from 140829678 bytes)") + } + if strings.Contains(html, "Repo Size 0 MB") || strings.Contains(html, ">0 MB<") { + t.Error("the card still renders the dead `repo_size_mb` field as 0 MB") + } + // `integrity_ok` has no producer anywhere in the controller — NotifyIntegrityOK and + // NotifyIntegrityFailed exist and are called from nowhere. A row that always reads "Unknown" is + // not information, and one that could read "OK" from an unwritten field would be a lie. + if strings.Contains(html, "Integrity") { + t.Error("the card still shows an Integrity row — nothing in the controller produces it") + } +} + +// The three-way ruling, which is the reason this card needed Go and not a template {{if}}. +func TestBackupCard_ThreeWayRuling(t *testing.T) { + t.Run("never measured shows a dash, never 0", func(t *testing.T) { + // A box with an off-site tier that has never read its repository. Reporting 0 here would + // claim the repository is EMPTY, which is not what is known — R-225's exact defect, which + // was measured live over a store that really held snapshot f3d9cd67. + html := renderWithReport(t, `{"backup":{"enabled":true}, + "offsite":{"enabled":true,"last_status":"ok","snapshot_count":0,"repo_size_bytes":0}}`) + if !strings.Contains(html, "never been") { + t.Error("an unmeasured repository is not called out as unmeasured — the operator reads " + + "the dash as a formatting quirk rather than as missing knowledge") + } + if strings.Contains(html, ">0<") { + t.Error("an unmeasured repository rendered a literal 0 snapshot count — zero is a claim " + + "about the repository's contents and nothing here justifies making it") + } + }) + + t.Run("measured empty is stated, not hidden", func(t *testing.T) { + // The opposite news, and it must be sayable: the repository was read and really holds + // nothing. If this rendered a dash too, the field would be pointless. + html := renderWithReport(t, `{"backup":{"enabled":true}, + "offsite":{"enabled":true,"last_status":"ok","snapshot_count":0,"repo_size_bytes":0, + "stats_known":true}}`) + if strings.Contains(html, "never been") { + t.Error("a MEASURED empty repository was reported as unmeasured — the operator never " + + "learns that a customer genuinely has zero off-site snapshots, which is an alarm") + } + if !strings.Contains(html, ">0<") { + t.Error("a measured-empty repository did not render its 0 — measured zero is knowledge " + + "and must be shown") + } + }) + + t.Run("no offsite object says so instead of showing numbers", func(t *testing.T) { + html := renderWithReport(t, `{"backup":{"enabled":true}}`) + if !strings.Contains(html, "No off-site data reported") { + t.Error("a report with no `offsite` object did not say so — the same rule offsiteBoxTile " + + "already follows: absent, never a zeroed tile") + } + if strings.Contains(html, "Off-site snapshots") { + t.Error("numbers were rendered for a customer whose controller has never reported any") + } + }) + + t.Run("a disabled tier with a declared state names the blocker", func(t *testing.T) { + html := renderWithReport(t, `{"backup":{"enabled":true}, + "offsite":{"enabled":false,"state":"needs_credential"}}`) + if !strings.Contains(html, "needs_credential") { + t.Error("the declared state was dropped — 'off-site not enabled' and 'off-site blocked " + + "waiting for a credential' are different operator actions and must read differently") + } + }) +} + +// A pre-v0.225.0 controller sends no `stats_known` key. Absence must degrade to UNKNOWN, never to +// "empty" — otherwise upgrading the hub before the fleet would tell the operator that every +// un-upgraded customer has zero backups. +func TestBackupCard_OldControllerDegradesToUnknownNotEmpty(t *testing.T) { + html := renderWithReport(t, `{"backup":{"enabled":true}, + "offsite":{"enabled":true,"last_status":"ok","last_success":"2026-08-30T02:17:19Z", + "snapshot_count":67,"repo_size_bytes":140829678,"quota_gb":50}}`) + + if !strings.Contains(html, "never been") { + t.Error("a report without stats_known was treated as authoritative — the hub cannot know " + + "whether an old controller's counts were ever read, and must say so") + } + // The rest of the card must still work: last successful run is independent of stats_known. + if !strings.Contains(html, "Last successful run") { + t.Error("the whole off-site section vanished for an old controller") + } +} + +func TestFmtBytesAuto_ScalesInsteadOfCollapsingToZero(t *testing.T) { + // The Offsite page's fmtBytesGB renders demo-hp's real size as "0.1 GB". On a card whose entire + // defect was under-reporting a real backup, that reads as "nearly nothing" — which is why this + // formatter exists rather than reusing that one. + for _, tc := range []struct{ in int64; want string }{ + {140829678, "134.3 MB"}, // demo-hp, measured + {0, "0 B"}, + {1 << 10, "1.0 KB"}, + {1 << 20, "1.0 MB"}, + {1 << 30, "1.0 GB"}, + {1 << 40, "1.00 TB"}, + } { + if got := fmtBytesAuto(tc.in); got != tc.want { + t.Errorf("fmtBytesAuto(%d) = %q, want %q", tc.in, got, tc.want) + } + } +} diff --git a/hub/internal/web/templates/customer_unified.html b/hub/internal/web/templates/customer_unified.html index 12868d8c..e4114b6b 100644 --- a/hub/internal/web/templates/customer_unified.html +++ b/hub/internal/web/templates/customer_unified.html @@ -271,28 +271,58 @@ {{end}} - +

Backup

- {{with .Report.backup}} + {{with .BackupCard}}
- Enabled - {{if index . "enabled"}}Yes{{else}}No{{end}} + App data (local) + {{if .LocalEnabled}}Enabled{{else}}Disabled{{end}}
- Snapshots - {{index . "snapshot_count"}} -
-
- Repo Size - {{index . "repo_size_mb"}} MB -
-
- Integrity - {{if index . "integrity_ok"}}OK{{else}}Unknown{{end}} + Last DB dump + {{if .LastDBDump}}{{.LastDBDump}}{{else}}—{{end}}
+ + {{if not .OffsiteReported}} +

No off-site data reported — the controller has never sent an + offsite object (no off-site tier, or a controller older than v0.109.0). + This is not the same as "no backups"; it means this page cannot say.

+ {{else if not .OffsiteEnabled}} +

Off-site backup is not enabled for this customer.{{if .OffsiteState}} + Declared state: {{.OffsiteState}} — the box is waiting on a credential + or recovery key and cannot back up off-site until it has one.{{end}}

+ {{else}} +
+
+ Off-site snapshots + {{if .StatsKnown}}{{.Snapshots}}{{else}}—{{end}} +
+
+ Repo size + {{if .StatsKnown}}{{.RepoSize}}{{else}}—{{end}} +
+
+ Last successful run + {{.LastSuccess}} +
+ {{if .QuotaStr}} +
+ Soft quota + {{.QuotaStr}} +
+ {{end}} +
+ {{if not .StatsKnown}} +

Snapshot count and repository size have never been + measured on this box — shown as — rather than 0, because zero would + claim the repository is empty and that is not what is known. They fill in after the next + off-site run reads the repository.

+ {{end}} + {{end}} {{end}}
{{end}} diff --git a/manifests/hub.yaml b/manifests/hub.yaml index d27bbd58..9f6c3e1b 100644 --- a/manifests/hub.yaml +++ b/manifests/hub.yaml @@ -125,7 +125,7 @@ spec: spec: containers: - name: hub - image: gitea.dooplex.hu/admin/felhom-hub:0.108.0 + image: gitea.dooplex.hu/admin/felhom-hub:0.109.0 ports: - containerPort: 8080 name: http