v0.148.0 — coherent snapshot pairs + an offsite restore that actually restores (R-43 + R-44)
Closes the two findings from DIAG-immich-restore-2026-07-19. Viktor deleted 11
immich photos to test offsite restore; both runs flashed success and the photos
stayed gone. Two independent defects.
R-43 — no offsite path could restore a database. All three buttons were
file-only: the two "visszaállítás" actions staged to a scratch folder and never
touched postgres, and place-to-live merged only MISSING files. For a DB-indexed
app the bytes returned and the app still could not see them. The dump was
carried INTO every snapshot and could never be replayed OUT of one.
New ReconstituteFromOffsite (/backup/offbox/reconstitute): safety dump → stop →
files overwritten to the snapshot version → start → the snapshot's own dump
replayed → health wait. Two invariants:
- nothing is ever deleted (-a, no --ignore-existing, no --delete): a file
created after the snapshot survives as an extra;
- the undo exists before the act — the pre-restore- dump is verified ON DISK
before anything is stopped, overwritten or replayed; if it cannot be taken
the operation refuses with zero changes.
The replay reads the SCRATCH unit: the live unit is never overwritten, so
replaying from it would replay the current DB over itself and restore nothing.
R-44 — a manual push shipped an unrefreshed dump (up to ~24h old). That day's
predated the customer's account by four hours and probed to asset:0/user:0/
album:0 inside 52MB whose bulk was immich's shipped geodata. Every run, manual
AND nightly, now refreshes dumps + units BEFORE capturing. Order is the
mechanism: the gap can only ADD files the DB does not reference yet, never
remove one it does. Manifests carry offsite_run_id + dumps_at, so coherence is
verifiable at restore time rather than assumed; the periodic refresh carries a
prior stamp forward and never invents one.
Honesty surfaces, all warn-level and none a gate: unstamped (pre-v0.148) pairs
report their skew, ValidateDump gained an EXACT-match accounts-table sniff for
customer-empty dumps, the completion flash states an outcome instead of a
mechanism, and the missing-only button now says what it does NOT do.
11 tests; 5 red-proofs run and reverted. Two of those found real test weaknesses
rather than confirming strength — the first undo mutation was caught by a second
guard, and the first table-matching test did not discriminate between the two
matchers at all. Both tests were rewritten to the cases that separate them.
NOT in scope: R-41's catalog invariant check, nightly cadence, retention, quota
math, tier-2, and v0.147.x progress semantics beyond one added phase line.
Live acceptance (§9) has NOT run: no capability-map flip, customer-restore row
stays MISSING, R-3 stays DRAFT.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01P9Nn14TWGzKoqAJAiVwC2s
This commit is contained in:
@@ -1,170 +1,85 @@
|
||||
# REPORT — controller v0.147.0 → v0.147.3: feedback slice 1 (2026-07-19)
|
||||
# REPORT — v0.148.0: coherent snapshot pairs + a real offsite restore (R-43 + R-44)
|
||||
|
||||
**Baseline v0.146.0 → LIVE v0.147.3 on demo guest 9201.** Green at every version:
|
||||
`go build ./... && go vet ./... && go test ./...` (23 packages) plus `template_id_gate`,
|
||||
`emoji_gate`, `native_confirm_gate`, `offbox_rename_gate`, `app_row_dedup_gate`, `mojibake_gate`.
|
||||
**Date:** 2026-07-19 · **Repo:** felhom-controller (v0.147.3 → **v0.148.0**) · Trunk, pushed to `main`.
|
||||
|
||||
> `docker_run_volume_path_gate` fails on `internal/appexport/estimate.go:179`. **Pre-existing on
|
||||
> HEAD and untouched by this release** — verified by stashing this work and re-running the gate.
|
||||
> Not fixed here (out of scope, and it is someone's call whether that `-v :ro` du probe gets an
|
||||
> allowlist entry or a rewrite).
|
||||
Closes the two findings from `felhom.eu/documentation/audits/DIAG-immich-restore-2026-07-19.md`.
|
||||
|
||||
## The problem
|
||||
## What was broken
|
||||
|
||||
The systemic complaint, twice in one evening: **you press a button and nothing happens.** No
|
||||
progress, no ETA, no named result. Three worst offenders fixed on the two patterns already in the
|
||||
codebase — the deploy 3-step panel and the storage-init status poll. **No new framework**; that is
|
||||
now ROADMAP **R-45**, because three targeted cards were worth shipping tonight and a framework was
|
||||
not.
|
||||
Viktor deleted 11 immich photos to test offsite restore. Both restore runs flashed success; the
|
||||
photos stayed gone. Two independent defects:
|
||||
|
||||
Three of the four versions below exist because the cards were watched against **real runs on the
|
||||
demo box**. Each follow-up is a thing no unit test would have surfaced.
|
||||
- **R-43** — no offsite path could restore a database. All three buttons were file-only, so for a
|
||||
DB-indexed app (most of the catalog) the bytes came back and the app still could not see them.
|
||||
- **R-44** — a manual push shipped whatever dump the 02:30 local run had left. That day's dump
|
||||
predated the customer's account by four hours and contained `asset: 0 / user: 0 / album: 0`, in a
|
||||
52 MB file whose bulk was immich's shipped geodata tables.
|
||||
|
||||
---
|
||||
## What shipped
|
||||
|
||||
## 4a — a verification restore names its result
|
||||
**P1 — coherence pre-phase.** Every offsite run (manual AND nightly) refreshes dumps + recovery
|
||||
units before capturing. Each manifest carries `offsite_run_id` + `dumps_at`. The periodic refresh
|
||||
carries a prior stamp forward and never invents one. A dump failure is a loud WARN that does not
|
||||
abort the push (data-first). New `dump` progress phase.
|
||||
|
||||
The completion flash said the app had been restored „ellenőrző mappába a meghajtón". *Which* folder,
|
||||
on *which* drive, was invisible — so the customer could not go and look at the thing they had just
|
||||
asked for. And nothing anywhere listed what these restores had accumulated, so copies piled up and
|
||||
the only way to find them was SSH.
|
||||
**P2 — honesty surfaces.** `ValidateDump` gained an exact-match accounts-table sniff
|
||||
(`LooksEmpty`); the confirm shows dump age, an unstamped-pair warning and the empty-dump warning.
|
||||
All warn-level, none a gate.
|
||||
|
||||
- The flash now carries the **full path**.
|
||||
- The restore page gained **„Meglévő ellenőrző másolatok"** — app · size · date · path — each with a
|
||||
double-confirmed **„Másolat törlése"** (the house inline `felhomConfirm` idiom, never native
|
||||
`confirm()`).
|
||||
- `backups/offsite-restore` was open-coded in three places; it now has one home
|
||||
(`offsiteRestoreRootFor`). A test pins the path in the flash to the path in the listing, so the
|
||||
customer can never be told about a directory the page cannot show or remove.
|
||||
**P3 — `ReconstituteFromOffsite`** (`/backup/offbox/reconstitute`): safety dump → stop → files
|
||||
overwritten → start → snapshot's dump replayed → health wait. Outcome-shaped flash. Two invariants:
|
||||
nothing is ever deleted (`-a`, no `--ignore-existing`, no `--delete`), and the `pre-restore-` undo
|
||||
is verified on disk before anything is touched — otherwise refuse with zero changes.
|
||||
|
||||
**The delete is the only one this release adds**, so it names a **stack, never a path**: the Manager
|
||||
resolves that name inside a root it computed itself and refuses anything landing outside.
|
||||
Red-proofed — neutralise the name guard and `stack: ""` resolves to the offsite-restore **root** and
|
||||
`os.RemoveAll` takes every copy with it. Every refusal is asserted as a **non-effect** (the
|
||||
neighbouring copy and the live data are still on disk afterwards).
|
||||
**P4 — tests.** 11 new; full suite green: `go build ./... && go vet ./... && go test ./...`.
|
||||
Design gates pass: template_id, emoji, native_confirm, offbox_rename, app_row_dedup, mojibake.
|
||||
|
||||
Honest note: the `HasPrefix` containment check behind that name guard could **not** be red-proofed
|
||||
independently — with `isSafeStackName` in front of it, no input the API accepts can reach it with an
|
||||
escaping path. It is kept as defence-in-depth and labelled in the test file as unproven-by-test
|
||||
rather than dressed up as a red-proof it does not have.
|
||||
> `docker_run_volume_path_gate` still fails on `internal/appexport/estimate.go:179`. **Pre-existing
|
||||
> on HEAD and untouched by this release** — same finding as the v0.147.x report, same file, not a
|
||||
> path this work goes near.
|
||||
|
||||
### Live (endpoint-exact, through the real handlers)
|
||||
## Red-proofs — 5 run and reverted
|
||||
|
||||
Real verification restore of `calibre-web`, then the guard matrix:
|
||||
| Mutation | Result |
|
||||
|---|---|
|
||||
| Replay removed | `0 DB dump(s) replayed` → FAIL |
|
||||
| Capture moved before the dump | `got [capture dump]` → FAIL |
|
||||
| Both undo guards removed | no refusal → FAIL |
|
||||
| Substring table matching | join table mistaken for accounts; `UserRows = 4, want 1` → FAIL |
|
||||
| Buffer-exceeding rows uncounted | wide row sniffs as empty → FAIL |
|
||||
|
||||
```
|
||||
message: "A(z) calibre-web visszaállítva ellenőrző mappába:
|
||||
/mnt/felhom-drives/hdd_1/backups/offsite-restore/calibre-web
|
||||
(a meglévő adatok változatlanok)."
|
||||
listing: calibre-web · 430.4 KB · …/offsite-restore/calibre-web · [Másolat törlése]
|
||||
**Two of these found real test weaknesses rather than confirming strength**, and both tests were
|
||||
rewritten:
|
||||
|
||||
stack=../primary -> refused; copy AND backups/primary both still present
|
||||
missing confirm -> refused; nothing changed
|
||||
real delete -> copy gone; backups/primary SURVIVES; page -> „Nincs ellenőrző másolat"
|
||||
```
|
||||
- The first undo mutation still refused — a second guard (`os.Stat` on the empty path) held. The
|
||||
red-proof only goes red when BOTH guards are removed, which is now recorded in REUSE.md.
|
||||
- The first table-matching test did not discriminate between the two matchers at all: its aggregate
|
||||
row count was identical either way. Rewritten to the case that separates them — an app with no
|
||||
accounts table but with `user_metadata` / `album_user` / `user_audit`, where a substring match
|
||||
would shout "your backup looks empty" on every restore of a healthy app.
|
||||
|
||||
---
|
||||
## Deliberately NOT done
|
||||
|
||||
## 4b — Megosztás enable shows what it is waiting for
|
||||
Per §10 of the task: no catalog-wide invariant check (stays R-41), no changes to nightly cadence,
|
||||
retention, quota math or tier-2, and no touching v0.147.x progress semantics beyond the one added
|
||||
phase line. The missing-only button's own zero-file flash is untouched — that belongs to the v0.147
|
||||
feedback arc, not R-43/R-44. Its label now states what it does *not* do, which is in scope.
|
||||
|
||||
Enabling sharing ran `ReconcileSamba()` **synchronously inside the POST handler**. On a box whose
|
||||
golden had not baked `felhom-samba` that is `compose up -d` pulling ~100MB from a private registry:
|
||||
minutes of an apparently-hung form post, then „Beállítás mentve." whether or not anything came up.
|
||||
## Live acceptance — NOT YET DONE (§9, needs Viktor)
|
||||
|
||||
Detached + polled now, with a card that distinguishes **„képfájl letöltése"** (image genuinely
|
||||
absent — the multi-minute case) from **„indítás"** (already baked — seconds). The distinction is
|
||||
decided *before* the work starts, because afterwards the image is always present and the card could
|
||||
never truthfully say „letöltés".
|
||||
The genuine red-proof is the joint run over the tunnel and has not happened: upload → „Távoli mentés
|
||||
most" → empty the trash for real → „Teljes visszaállítás (fájlok + adatbázis)" → photos visible in
|
||||
the immich timeline. Until that evidence exists:
|
||||
|
||||
- Success is **probed, not inferred** — `compose up -d` exits 0 on a crash-loop, so the terminal
|
||||
state is container liveness.
|
||||
- `nil` from reconcile also covers "deliberately deployed nothing because there is no household
|
||||
password yet"; that now gets its own message instead of a card spinning forever.
|
||||
- The **password** form starts the same job — with `UserSet` false reconcile deploys nothing, so on a
|
||||
fresh box *that*, not the enable toggle, is where the pull actually happens.
|
||||
- the capability-map offsite row stays **PARTIAL / scope-contested** (no PROVEN-LIVE flip),
|
||||
- the customer-restore row stays **MISSING**,
|
||||
- R-3 stays **DRAFT**.
|
||||
|
||||
**Test scope, stated honestly:** the unit tests cover the single-flight slot, snapshot isolation and
|
||||
the phase vocabulary. They do **not** drive `runSambaEnsureJob` end-to-end — the Server field is the
|
||||
concrete `*stacks.Manager`, not an interface, and introducing one purely for this card was more churn
|
||||
than the slice warranted. Pulling-vs-starting and the probed terminal state are covered by live
|
||||
validation instead. That seam belongs to R-45.
|
||||
**Floor raise to v0.148.0 is Viktor's click and must come BEFORE the acceptance run.**
|
||||
|
||||
Live: `/sharing/status` returns `{"ok":true,"data":{"phase":"running","running":true,"error":""}}`;
|
||||
the page carries the card and the poll.
|
||||
## Open question for Viktor
|
||||
|
||||
---
|
||||
|
||||
## 4c — „Távoli mentés most" streams real progress
|
||||
|
||||
restic was already reporting bytes and percentages; the runner seam used `CombinedOutput()` and threw
|
||||
them away. The manual run now passes `--json`, scans stdout line-by-line, and the page shows total
|
||||
bytes, percent and the app being pushed. **Manual only** — the nightly stays silent, pinned by a test
|
||||
that fails if the scheduled path ever passes `--json` or publishes progress.
|
||||
|
||||
The poll also now **arms unconditionally**. It used to start only if the page already rendered
|
||||
„Fut…", which loses a race the manual trigger always runs: the POST redirects and the page renders
|
||||
before the detached goroutine writes `LastStatus=running`, so the poll never armed and the customer
|
||||
watched a static page during the very run they had just started.
|
||||
|
||||
Red-proofed twice: break the parser -> the percent assertion fails; drop the wiring -> the `--json`
|
||||
assertion fails.
|
||||
|
||||
### What three live runs actually taught (v0.147.1 -> .3)
|
||||
|
||||
**Run 1 — immich reported `0%` for 40+ seconds.** The parser was not broken. On an incremental run
|
||||
where nothing changed restic transfers nothing: `bytes_done` is `omitempty`, so it is not even in the
|
||||
JSON, and `percent_done` stays 0. Confirmed by capturing the real schema from
|
||||
`backup --dry-run --json` on restic 0.14.0 in the controller image rather than guessing; those lines
|
||||
are now quoted verbatim in the type's doc comment. **A byte-only bar is indistinguishable from a hang
|
||||
in the common case** — shipping it would have traded "no feedback" for "feedback that says 0% and
|
||||
looks stuck". -> **v0.147.1** parses and publishes `files_done`/`total_files`.
|
||||
|
||||
**Run 2 — bookstack showed clean byte progress (100%, 154.0 MB, 7/7), immich sat at `1 of 46 files,
|
||||
0 bytes` for 42 seconds.** restic 0.14 only counts a file when it **completes**, so an app dominated
|
||||
by one large archive (immich's ~430MB volume tar) freezes *both* counters. No percentage can move.
|
||||
-> **v0.147.2** shows what restic *does* keep reporting: the file being processed and elapsed
|
||||
seconds. „1 / 46 fájl (430.2 MB) · feldolgozás alatt: immich_upload.tar · 42 mp" is a different
|
||||
message from „0%".
|
||||
|
||||
**Run 3 — the per-app legs finished in ~15s; the remaining 40 of a 57-second run was the shares leg
|
||||
and `forget --prune`,** during which the card sat frozen on „calibre-web — 8 / 8 fájl". The same
|
||||
silence, relocated to the end of the run. -> **v0.147.3** adds a run **phase**; the post-app stages
|
||||
announce themselves and app-scoped counters clear when a phase starts.
|
||||
|
||||
Final live trace — the whole 57-second run is narrated, no frozen state:
|
||||
|
||||
```
|
||||
t+6s app=immich
|
||||
t+9s app=bookstack
|
||||
t+12s app=calibre-web
|
||||
t+15s phase=shares
|
||||
t+18s ... t+54s phase=retention
|
||||
t+57s done
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Also: golden/controller infra-image drift closed at the source
|
||||
|
||||
`infra.Images()` derives the list from the existing pins; `--print-infra-images` prints it; the golden
|
||||
bake now asks the controller binary it is about to bake instead of carrying its own copy. That copy
|
||||
had already drifted — `felhom-samba` was never added to it, so the golden baked 3 of 4, **which is
|
||||
why enabling Megosztás pulled at runtime in the first place**. A test parses the const block out of
|
||||
the source with `go/ast` and fails if a pin is added without reaching `Images()`; red-proofed.
|
||||
|
||||
Verified against the real published image on the live box — all four returned.
|
||||
|
||||
## Validation method
|
||||
|
||||
Live validation was **endpoint-exact** (the exact URLs the UI posts to, through the real handlers and
|
||||
the real restic/samba paths), not browser click-through: the controller UI is password-gated and I do
|
||||
not enter credentials into web forms. No server logic was skipped — only rendering. The rendered
|
||||
markup was checked by fetching the pages themselves and asserting on the emitted HTML.
|
||||
|
||||
## Not done
|
||||
|
||||
- **Floor not raised** — Viktor decides floor timing.
|
||||
- No golden rebuild, no manifest saves, nothing on Peti's arc.
|
||||
- No SMB server-side changes — the `\\FELHOM` access issue stays client-side diagnosis first.
|
||||
- ROADMAP: **R-45** (unified async-job feedback) and **R-46** (verification-copy browse + expiry)
|
||||
registered in `felhom.eu/documentation/backlog/ROADMAP.md`.
|
||||
The capability-map row at `00-capability-map.md:61` claims PROVEN-LIVE citing `CAMPAIGN-6D`'s
|
||||
"immich end-to-end from offsite alone". This diagnosis contradicts that phrase. I flagged it
|
||||
scope-contested rather than downgrading a proven row — the ruling (did 6D exercise the DB half, or
|
||||
only the file half?) is still outstanding and is not something this implementation can settle.
|
||||
|
||||
Reference in New Issue
Block a user