docs: DIAG-sharing addendum — S-1/S-2/S-4-core/S-5 shipped in controller v0.151.0; S-3 stays open
Also records against R-45 the lesson the loop taught: a phase a client answers with a one-shot action must be an EDGE the registry serves once, never a level synthesised into the phase channel.
This commit is contained in:
@@ -216,11 +216,11 @@ starts with "which network am I actually on".
|
||||
|
||||
| # | Sev | Finding | Evidence | Proposed fix | GO needed? |
|
||||
|---|---|---|---|---|---|
|
||||
| **S-1** | **HIGH** | `/sharing` reload-loops forever for every customer with sharing enabled — the page is unusable. `sharingStatusHandler` reports `running` as a level; the JS treats it as a one-shot success edge and calls `location.reload()`. | `sharing_handlers.go:246` (blame `b5d78d1`), `sharing.html` L320–326, 6× identical `phase:"running"` poll | **XS.** Make the JS reload only when it has *observed a transition* — latch a `sawInFlight` flag on the first tick and reload on `running` only if it is set; otherwise just paint the success alert and stop. Server-side alternative (also XS): report the coerced level as a distinct phase (`running_steady`) the JS renders but never reloads on. Prefer the JS latch — it keeps the honest "liveness wins" contract the handler comment states. **Needs a red-proof:** a template/JS test or handler test asserting a fresh page load with a live container does not schedule a reload. | no — plain bug fix |
|
||||
| **S-2** | **MED** | The UI never tells the customer *where* to connect. `sharing.html` L32/L61 show only `\\FELHOM`; there is no IP anywhere on the page, so a customer who cannot resolve the name has nothing to fall back on and will guess (Viktor guessed the Proxmox host IP). | `sharing.html` L30–32, L61; symptom 1 itself | **S.** Render the guest's current LAN address beside the name: `\\FELHOM` *(vagy `smb://192.168.0.104`)*, sourced from the same place the dashboard already knows the guest IP. Add a one-line macOS hint (Finder → Ugrás → Csatlakozás a szerverhez). | no |
|
||||
| **S-3** | **MED** | No mDNS/Bonjour advertisement. `smbd + nmbd + wsdd` covers Windows (WSD) and legacy NetBIOS, but macOS Finder's preferred path is Bonjour — `FELHOM.local` does not resolve and the share never appears in the Finder sidebar. Likely the whole of symptom 1b. | nothing on udp/5353 (§ Phase A); container log daemon list; `infra/samba.go` header comment citing the R-6 spike | **S–M.** Add avahi-daemon (or `wsdd`'s mDNS sibling) to the felhom-samba image publishing `_smb._tcp`, host-net so multicast reaches the LAN. Re-runs the R-6 spike's constraint set — treat as a slice, not a hotfix. Confirm with the Mac matrix first (below) so we fix the mechanism that is actually failing. | yes — image change + republish |
|
||||
| **S-4** | **LOW** | Success-state semantics: the ensure job's phase never returns to `idle` after a terminal phase, and `SambaRunning()` masks that anyway. Harmless today only because S-1's fix removes the consumer that cared. Worth folding into the **unified async-job feedback** roadmap item the `samba_ensure_job.go` header already names. | `samba_ensure_job.go` (no reset path); handler line 246 | Fold into the roadmap item; no standalone task. | no |
|
||||
| **S-5** | **INFO** | The guest holds its LAN address by **DHCP** (`ip=dhcp`, lease ~54 min at probe). Any printed IP is not stable across a lease change or a site move, which weakens S-2 unless the page re-reads it live (it should — render it, never persist it). | `pct config 9201`; `valid_lft 3251sec` | Render live, never cache. Consider a DHCP reservation at the customer's router as a documented install step. | no |
|
||||
| **S-1** — **FIXED v0.151.0** | **HIGH** | `/sharing` reload-loops forever for every customer with sharing enabled — the page is unusable. `sharingStatusHandler` reports `running` as a level; the JS treats it as a one-shot success edge and calls `location.reload()`. | `sharing_handlers.go:246` (blame `b5d78d1`), `sharing.html` L320–326, 6× identical `phase:"running"` poll | **XS.** Make the JS reload only when it has *observed a transition* — latch a `sawInFlight` flag on the first tick and reload on `running` only if it is set; otherwise just paint the success alert and stop. Server-side alternative (also XS): report the coerced level as a distinct phase (`running_steady`) the JS renders but never reloads on. Prefer the JS latch — it keeps the honest "liveness wins" contract the handler comment states. **Needs a red-proof:** a template/JS test or handler test asserting a fresh page load with a live container does not schedule a reload. | no — plain bug fix |
|
||||
| **S-2** — **SHIPPED v0.151.0** | **MED** | The UI never tells the customer *where* to connect. `sharing.html` L32/L61 show only `\\FELHOM`; there is no IP anywhere on the page, so a customer who cannot resolve the name has nothing to fall back on and will guess (Viktor guessed the Proxmox host IP). | `sharing.html` L30–32, L61; symptom 1 itself | **S.** Render the guest's current LAN address beside the name: `\\FELHOM` *(vagy `smb://192.168.0.104`)*, sourced from the same place the dashboard already knows the guest IP. Add a one-line macOS hint (Finder → Ugrás → Csatlakozás a szerverhez). | no |
|
||||
| **S-3** — **OPEN** | **MED** | No mDNS/Bonjour advertisement. `smbd + nmbd + wsdd` covers Windows (WSD) and legacy NetBIOS, but macOS Finder's preferred path is Bonjour — `FELHOM.local` does not resolve and the share never appears in the Finder sidebar. Likely the whole of symptom 1b. | nothing on udp/5353 (§ Phase A); container log daemon list; `infra/samba.go` header comment citing the R-6 spike | **S–M.** Add avahi-daemon (or `wsdd`'s mDNS sibling) to the felhom-samba image publishing `_smb._tcp`, host-net so multicast reaches the LAN. Re-runs the R-6 spike's constraint set — treat as a slice, not a hotfix. Confirm with the Mac matrix first (below) so we fix the mechanism that is actually failing. | yes — image change + republish |
|
||||
| **S-4** — **CORE SHIPPED v0.151.0** | **LOW** | Success-state semantics: the ensure job's phase never returns to `idle` after a terminal phase, and `SambaRunning()` masks that anyway. Harmless today only because S-1's fix removes the consumer that cared. Worth folding into the **unified async-job feedback** roadmap item the `samba_ensure_job.go` header already names. | `samba_ensure_job.go` (no reset path); handler line 246 | Fold into the roadmap item; no standalone task. | no |
|
||||
| **S-5** — **SHIPPED v0.151.0** | **INFO** | The guest holds its LAN address by **DHCP** (`ip=dhcp`, lease ~54 min at probe). Any printed IP is not stable across a lease change or a site move, which weakens S-2 unless the page re-reads it live (it should — render it, never persist it). | `pct config 9201`; `valid_lft 3251sec` | Render live, never cache. Consider a DHCP reservation at the customer's router as a documented install step. | no |
|
||||
|
||||
**Recommended packaging:** S-1 alone as an immediate patch task (it bricks a shipped page). S-2 with
|
||||
it if the guest IP is already available to the sharing handler — same page, same release. S-3 as its
|
||||
@@ -265,3 +265,35 @@ for the `/sharing/status` polls, which mutates nothing.
|
||||
is off the DooPlex segment — a handy check for the rest of the trip.
|
||||
* No secrets appear in this document: the household SMB password and the controller login password
|
||||
are stored out-of-band.
|
||||
|
||||
---
|
||||
|
||||
## Addendum — 2026-07-20, controller v0.151.0 (S-1, S-2, S-4-core, S-5 closed)
|
||||
|
||||
Shipped and live on demo guest 9201 the same day this diagnosis was written. Controller commit
|
||||
`badf17b`, image `gitea.dooplex.hu/admin/felhom-controller:0.151.0`, `Up (healthy)`.
|
||||
|
||||
| # | Status | What changed |
|
||||
|---|---|---|
|
||||
| **S-1** | **FIXED v0.151.0** | The `idle`->`running` coercion is gone from `sharingStatusHandler`. The duty it was written for — "liveness must never be contradicted by a missing job" — was always discharged by the `running` LEVEL field beside it, and is now pinned by its own named regression test instead of by a phase-channel side effect. `sharing.html`'s `<script>` block is **byte-identical** to v0.150.0: the client contract was proven fixed server-side, not papered over. |
|
||||
| **S-2** | **SHIPPED v0.151.0** | New „Csatlakozás a megosztáshoz" card: the Windows form, the Mac form, and the direct `smb://<IP>` fallback. Rendered only while sharing is enabled; the address line is absent whenever no address can be derived. |
|
||||
| **S-4** | **CORE SHIPPED v0.151.0** | `consumeIfRunning` serves a terminal `running` exactly once, so a REAL bring-up cannot re-arm the reload on the page it just caused — without it the loop returns after every future image update. `failed`, `needs_password` and in-flight phases are never consumed. **The unified async-job feedback layer stays the roadmap item (R-45)**; this is the minimal contract fix, not the framework. |
|
||||
| **S-5** | **SHIPPED v0.151.0** | The address is derived per render from the samba container's netns and stored nowhere — no settings key, no memo, no package-level cache. Deriving it *through the samba container* also avoids the trap that made the card necessary: the controller is on a docker bridge, so `net.InterfaceAddrs()` would answer `172.x`. |
|
||||
| **S-3** | **OPEN** | Unchanged — still no mDNS/Bonjour advertisement, and still awaiting the two Mac-side probes from the matrix above (`smbutil lookup FELHOM`, `dns-sd -B _smb._tcp`) before an image change is justified. Deliberately out of scope for v0.151.0. |
|
||||
|
||||
### Live validation (endpoint-level — the exact endpoints the UI invokes; no browser on DooPlex)
|
||||
|
||||
```
|
||||
GET /sharing/status x3 -> {"data":{"error":"","phase":"idle","running":true},"ok":true} (all three)
|
||||
was {"phase":"running","running":true} on every call before the fix
|
||||
GET /sharing -> connect card present; smb://FELHOM and smb://192.168.0.104
|
||||
(192.168.0.104 = the guest's real DHCP lease, derived, not a literal)
|
||||
```
|
||||
|
||||
The `phase:"idle"` triple is the server-side proof that the loop's trigger is gone: the client's
|
||||
reload path is reachable only from `phase === "running"`. The human confirmation that the page sits
|
||||
still is Viktor's to give — a browser is the one method not available here.
|
||||
|
||||
**Tests:** 23/23 packages green (twice). Three red-proofs recorded — reinstating the coercion,
|
||||
deleting the serve-once clear, and memoizing the derived address each turned the corresponding test
|
||||
red, then were restored.
|
||||
|
||||
@@ -88,7 +88,7 @@
|
||||
| R-42 | **Ruling needed: do DB/cache sidecar majors follow the app, or the newest tag?** Campaign 7 deliberately did NOT bump sidecar majors (postgres 16→18, redis 7→8, mariadb 11.6→12.3, postgis 16→17) while bumping ~40 app images to current. | S | **decision pending (Viktor)** | Origin: CAMPAIGN 7 §4. The case for not bumping: a DB major is a **data-plane decision the application owns** — immich proves it, upstream pins one specific tested `postgres:14-vectorchord…` build — and `postgres:16-alpine`/`redis:7-alpine` already track the newest patch inside their major, so they are not stale. The case for bumping: EOL majors eventually stop getting security patches, and "we never bump" silently becomes "we ship EOL databases". Suggested shape: per-app sidecar pin follows **upstream's own compose** where upstream publishes one, else stay within the current major and revisit at that major's EOL date |
|
||||
| R-43 | **[P2-HIGH] No offsite restore path can restore a database — offsite restore cannot reconstitute a DB-indexed app.** All three offsite buttons are file-only. „Ellenőrző visszaállítás" and „Teljes visszaállítás" restore into a **scratch/verify folder** and never load a dump into postgres (`controller/internal/backup/offbox_restore.go:171-232`, `:130-164`); „Helyreállítás az élő adatok közé (csak a hiányzó fájlok)" is `rsync -a --ignore-existing` over **mandatory userdata paths only** — it never touches a DB and never stops/starts the stack (`offbox_restore.go:346-421`). The DB dump is faithfully carried *into* the snapshot and can never be replayed *out of* it by any offsite action; the only code that replays into a live DB is `RestoreFromRecoveryUnit` (`backup/restore_unit.go:74`), reachable solely from the **local** restore panel (`web/handlers.go:1108`). Compounding: the `live recovery unit present — not overwriting` guard (`offbox_restore.go:400-405`) skips the **entire** unit — compose, manifest **and db-dumps** — whenever a local `backups/primary/<app>` exists, which is true on any box whose local backup has run, so on a normal box the offsite dump is never even staged to disk. | M | **SHIPPED controller v0.148.0 (2026-07-19) — live acceptance PENDING** | **SHIPPED:** new „Teljes visszaállítás (fájlok + adatbázis)" (`ReconstituteFromOffsite`, `/backup/offbox/reconstitute`): safety dump → stop → files overwritten to the snapshot version → start → the snapshot's own dump replayed (`reimportDBDumpsFrom`, reading the SCRATCH unit — the live unit is still never overwritten) → health wait. Two invariants, both red-proofed: **nothing is ever deleted** (`rsync -a`, no `--ignore-existing`, no `--delete`; post-snapshot files survive as extras) and **the undo exists before the act** (the `pre-restore-` dump is verified ON DISK before anything is stopped/overwritten/replayed; if it cannot be taken the operation refuses with zero changes). Outcome-shaped flash; the old missing-only button now states what it does NOT do. Deployed to demo guest 9201, healthy; page renders, button correctly gated behind a prepared full scratch. **STILL OPEN — the §9 live acceptance has NOT run** (upload → push → empty the trash for real → one button → photos back in the timeline). Until it does, no capability-map flip, the customer-restore row stays MISSING and R-3 stays DRAFT. — **Evidence: `audits/DIAG-immich-restore-2026-07-19.md` (live on demo-felhom).** Viktor deleted 11 immich photos to test offsite restore; both place runs merged **0 files** and flashed success. Root cause is two-layered — the UI delete only *trashed* the assets so nothing was missing to restore, **but a valid test would also have failed**: files return, the library stays empty, because immich's truth lives in postgres. **The customer-visible failure mode: correct bytes on disk and an app that cannot see them.** Severity case: this is the offsite tier — the one that survives losing the box — and most of the catalog is DB-indexed. Direction (needs a spec, do not improvise): either offsite gains a real DB-replay leg reusing `RestoreFromRecoveryUnit` behind a stop→restore→start sequence, or the offsite UI states plainly that it restores files only and routes the customer to the local recovery-unit flow for the DB. Pairs with R-44 (a replay leg is worthless while the dump is stale). Flips: the **customer-restore map row stays MISSING** and gains this as a note — the C6 evidence attempt found a product gap, which is the rehearsal system working |
|
||||
| R-44 | **[P2-HIGH] A manual offsite push ships an unrefreshed DB dump — "backed up now" is false for the DB half.** `offboxRunHandler` → `RunOffboxBackup` goes straight to the restic push and never calls `RunDBDumps` / `captureAllRecoveryUnits` (`controller/internal/web/offbox_handlers.go:203-227`, `backup/offbox.go:574-759`); the recovery unit merely **enumerates** existing dump filenames via `listFileNames`, never creates them (`backup/recovery_unit.go:105-106`). Dumps come only from the separate local `db-dump` daily at **02:30** (`cmd/controller/main.go:542`), with the scheduled offsite at 04:15 — so a *manual* run at any other hour ships a dump up to ~24 h old. **There is no freshness check and no RPO surface anywhere:** zero `RPO` hits across `controller/`; `offboxUnitTime` is only a two-drive tiebreak (`offbox.go:827-837`); the `DBValidationCache` exists (`backup.go:364-370`) but no offsite or restore path reads it. | S–M | **SHIPPED controller v0.148.0 (2026-07-19)** | **SHIPPED:** every offsite run — **manual AND nightly** — now refreshes the DB/volume dumps and recovery units (`offsitePreDump` → `runDBDumpsInternal`) BEFORE the restic capture, so each snapshot is an internally coherent `{DB@T, files@T}` bundle and retention becomes a history of restorable points. Order is the mechanism and is red-proofed (moving the capture first yields `[capture dump]`): the gap can only ADD files the DB does not reference yet, never remove one it does. This also makes the nightly ordering **structural** rather than a coincidence of two scheduler entries at 02:30 and 04:15. Each unit manifest carries `offsite_run_id` + `dumps_at`, so a pair's coherence is verifiable at restore time instead of assumed; the periodic refresh carries a prior stamp forward and never invents one. A dump-leg failure is a loud WARN that does NOT abort the push (data-first: a degraded backup beats none). Honesty surfaces, all warn-level and none a gate: an unstamped pre-v0.148 pair reports its skew in the confirm, and `ValidateDump` gained an **exact-match** accounts-table sniff for customer-empty dumps (a substring match on "user" would flag `user_metadata`/`album_user`/`user_audit` on every healthy single-user box — red-proofed). — **Evidence: `audits/DIAG-immich-restore-2026-07-19.md`.** Today's unit dump `immich-postgres.sql` (51 954 452 B, mtime **02:30 CEST**) probed to **`asset: 0 rows`, `user: 0 rows`, `album: 0 rows`** — the 52 MB is entirely immich's shipped `geodata_places`/`naturalearth_countries` reference data. It predates both the admin user (created 07:56:25) and the photos (07:57). Same for the unit's `immich_immich_postgres_data.tar` (323 MB, also 02:30). **A dump that looks substantial by size can contain zero customer content** — size is not a health signal, and nothing in the product says otherwise. **Latent hazard:** had a full restore actually loaded that dump it would have written an empty DB over the live one, destroying the trashed rows that were the only surviving recovery path. Direction: dump-before-push on manual runs (the honest fix), **or** an explicit RPO line in the UI („adatbázis-állapot: <dump time>") so the operator/customer can see what they are actually shipping. Cheap interim: surface dump mtime + row-count sanity from the existing `DBValidationCache` on `/backups/restore` |
|
||||
| R-45 | **[P2] Unified async-job feedback.** Every long operation invents its own progress surface, or none. Tonight produced three more one-off cards (v0.147.x: samba bring-up, offsite progress, restore result) on top of two existing patterns (deploy 3-step panel; storage-init/netstorage status poll). They agree on nothing: some use `{ok,data}` envelopes and some raw JSON, some poll 1 s / 1.5 s / 3 s, some are in-memory-only and lie after a restart, and each re-implements single-flight + snapshot + phase→Hungarian mapping. | M | idea | Origin: 2026-07-19 feedback slice 1 (controller v0.147.0). The cases to generalise from are all in-tree: `web/storage_init_job.go` (the best shape — acquire/release/set/snapshot), `web/netstorage_job.go`, `web/samba_ensure_job.go`, `backup/opstatus.go`, `backup/offbox_progress.go`. Shape: one job registry + one poll endpoint + one client-side renderer, phases declared per job. **Two lessons tonight that any framework must encode:** (1) a terminal state must be **probed, not inferred** — `compose up -d` exits 0 on a crash-loop; (2) a progress source that reports nothing is normal, not broken — restic reports 0 bytes for a whole incremental run, and a bar that sits at 0% is worse than no bar. Also fixes the restart hole: in-memory job state currently vanishes and the card silently disagrees with reality |
|
||||
| R-45 | **[P2] Unified async-job feedback.** Every long operation invents its own progress surface, or none. Tonight produced three more one-off cards (v0.147.x: samba bring-up, offsite progress, restore result) on top of two existing patterns (deploy 3-step panel; storage-init/netstorage status poll). They agree on nothing: some use `{ok,data}` envelopes and some raw JSON, some poll 1 s / 1.5 s / 3 s, some are in-memory-only and lie after a restart, and each re-implements single-flight + snapshot + phase→Hungarian mapping. | M | idea | Origin: 2026-07-19 feedback slice 1 (controller v0.147.0). The cases to generalise from are all in-tree: `web/storage_init_job.go` (the best shape — acquire/release/set/snapshot), `web/netstorage_job.go`, `web/samba_ensure_job.go`, `backup/opstatus.go`, `backup/offbox_progress.go`. Shape: one job registry + one poll endpoint + one client-side renderer, phases declared per job. **Two lessons tonight that any framework must encode:** (1) a terminal state must be **probed, not inferred** — `compose up -d` exits 0 on a crash-loop; (2) a progress source that reports nothing is normal, not broken — restic reports 0 bytes for a whole incremental run, and a bar that sits at 0% is worse than no bar. Also fixes the restart hole: in-memory job state currently vanishes and the card silently disagrees with reality **2026-07-20 — the first bill for NOT having this arrived, and it was customer-facing.** The samba card's poll (`web/samba_ensure_job.go` + `sharing.html`) mixed a job EDGE and a service LEVEL on one JSON field, and `/sharing` reload-looped at ~1.2 s for every customer with sharing enabled until controller v0.151.0 (`audits/DIAG-sharing-2026-07-20.md`, S-1/S-4). v0.151.0 fixed THAT card's contract only — the framework is still this item. **Third lesson for it to encode, beside the two already listed:** a phase a client answers with a one-shot action must be an EDGE the registry SERVES ONCE, and must never be synthesised from a level; if it can be re-read, it will be re-acted on. |
|
||||
| R-46 | **[P2] Verification copies need a customer-visible browse surface and an expiry.** v0.147.0 made them *visible* (listed with path/size/date, individually deletable) — but the customer still cannot LOOK INSIDE a verification restore to confirm the file they wanted is really there, which is the entire point of a verification restore, and nothing ever removes them. | S–M | idea | Origin: 2026-07-19 feedback slice 4a, registered as the explicit follow-up to it. Two gaps, deliberately designed together because they are the same object: (a) **the invisible-result gap** — a read-only browse of `backups/offsite-restore/<app>` (the FileBrowser infra stack already exists and already serves scoped roots, so this may be a mount rather than new code); (b) **the disk-lifecycle gap** — auto-expiry after N days with the count/size surfaced before it fires, so a drive is never quietly filled by verification restores nobody remembers taking. Pairs with R-43: a browse surface is also how a customer would discover that a DB-indexed app's files came back but the app still cannot see them |
|
||||
| R-47 | **[P2-HIGH] The DB replay races the application's own schema repair — a restore can abort half-applied.** The reconstitution starts the stack BEFORE replaying (ImportDump needs a live container), which hands the app a window to recreate schema objects the dump is about to create. Proven to the second on 2026-07-19: controller began the replay 10:58:25, **immich-server logged `Reindexing clip_index` → `Reindexed clip_index` at 10:58:33**, and the dump's own `CREATE INDEX clip_index` failed at 10:58:35 with `already exists` (exit 3, `ON_ERROR_STOP=1`). immich then reported **schema drift** — the indexes the aborted script never reached. | M | idea | **Evidence: `audits/DIAG-immich-restore-round2-2026-07-19.md` (H4, live on demo-felhom).** The photos survived only because `pg_dump` emits COPY data BEFORE CREATE INDEX, so the abort landed after the rows — **that ordering is luck, not design**: a collision earlier in the script aborts before the data and leaves a genuinely half-restored database, reported identically. **Class defect, not offsite-only:** the same start-then-replay shape is on the LOCAL path (`RestoreFromRecoveryUnit` → `RecreateStackFromUnit` → `reimportDBDumpsCtx`), so the local restore carries the same race. Direction (NOT decided — needs a spec): bring up the DB container alone for the replay and start the app only afterwards, or quiesce the app's schema management for the duration. Note `--clean --if-exists` + `ON_ERROR_STOP=1` are both CORRECT and should stay — the bug is the window, not the flags. Blocks the §9 acceptance and therefore the customer-restore map row |
|
||||
| R-48 | **[P2-HIGH] Restore controls are separable only by layout — and the difference between them is whether the data comes back.** The offsite restore row renders four buttons plus hint text into an overlapping, unreadable line, and the decisive second step („Teljes visszaállítás indítása") appears ONLY after „…előkészítése" was pressed, with no signposting that a second step exists or that the first one did nothing to live data. | M | idea | **Evidence: `audits/DIAG-immich-restore-round2-2026-07-19.md` (finding 1) — this is not theoretical: it is the CAUSE of the round-2 incident.** An operator who had read the code pressed the missing-only button instead of the full restore; the controller log shows `/backup/offbox/reconstitute` was never hit at all. The rule this establishes, worth stating once and applying beyond this page: **two adjacent controls whose difference is "your data comes back" vs "your data cannot come back" must not be distinguishable only by layout.** Direction (ruled in principle, spec rides v0.149): collapse to a single „Visszaállítás…" guided dialog — one intent, visible phases, the escrow-wizard precedent. Pairs with R-45 (the phases are exactly the async-feedback surface) and R-46 |
|
||||
|
||||
Reference in New Issue
Block a user