v0.147.0 — feedback slice 1: pressing a button says something

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 here (deploy 3-step panel, storage-init status poll).
No new framework — that is a ROADMAP item; three targeted cards ship tonight.

4a — a verification restore names its result. The flash said the app had been
restored "to a verification folder on the drive"; which folder, on which drive,
was invisible, so the customer could not go and look at what they had just asked
for. Full path now. The restore page gained a listing of existing verification
copies (app, size, date, path) — nothing anywhere showed these, so they piled up
and the only way to find them was SSH — each with a double-confirmed delete.

That delete is the only one this release adds, so it names a STACK, never a
path: the Manager resolves the name inside a backups/offsite-restore root it
computed itself and refuses anything landing outside. Red-proofed — neutralise
the name guard and stack:"" resolves to the offsite-restore ROOT and takes every
copy with it. Refusals are asserted as non-effects.

4b — Megosztás enable shows what it is waiting for. Enabling ran ReconcileSamba
synchronously inside the POST handler; on a golden without felhom-samba baked
that is compose pulling ~100MB, i.e. minutes of an apparently-hung form post
followed by "Beállítás mentve." whether or not anything came up. Detached +
polled now, distinguishing "képfájl letöltése" from "indítás" — decided BEFORE
the work starts, since afterwards the image is always present. Success is
probed, not inferred (compose up -d exits 0 on a crash-loop). The password form
starts the same job: with UserSet false reconcile deploys nothing, so on a fresh
box that is where the pull actually happens.

4c — "Távoli mentés most" streams real progress. restic was already reporting
bytes and percent; the runner seam used CombinedOutput() and discarded them. The
manual run now passes --json and scans stdout line-by-line: total bytes, percent,
current app. Manual only — the nightly stays silent, pinned by a test that fails
if it ever passes --json. The poll now arms unconditionally, closing a race the
manual trigger always ran: the redirect rendered before the goroutine wrote
LastStatus=running, so the poll never armed and the page sat static during the
very run just started. Red-proofed twice.

Also closes the golden/controller infra-image drift at the source: infra.Images()
derives from the existing pins and --print-infra-images exposes it, so the golden
bake can stop carrying its own copy. That copy had already drifted — felhom-samba
was never added, so the golden baked 3 of 4, which is why enabling Megosztás
pulled at runtime in the first place.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nn3VgQk9iwEGgyx6QJ2NvE

controller/internal/backup/offbox_progress_test.go

@@ -0,0 +1,228 @@
+package backup
+
+import (
+	"context"
+	"strings"
+	"testing"
+)
+
+// v0.147.0 slice 4c — live progress for a MANUAL offsite run.
+//
+// These tests exercise the WHOLE path a real run takes: a fake restic emits canned `--json` status
+// lines through the streaming seam, and the assertion is on what the poll surface
+// (OffboxProgressSnapshot) reports — not on the parser in isolation. A parser that works but is
+// never wired to the snapshot would leave the customer looking at the same silent spinner, which is
+// the bug being fixed.
+//
+// RED-PROOF (run manually, both confirmed to fail):
+//   1. break the parser — flip `s.MessageType != "status"` to `== "status"` in parseResticStatus:
+//      TestManualRunReportsParsedProgress fails ("percent = 0, want 42").
+//   2. drop the wiring — make resticBackupStep always delegate to resticStep:
+//      the same test fails (no --json, no stream, no snapshot).
+
+// jsonStatus is one restic --json status line.
+func jsonStatus(pct float64, done, total int64) string {
+	return `{"message_type":"status","percent_done":` + ftoa(pct) + `,"total_bytes":` + itoa(total) + `,"bytes_done":` + itoa(done) + `}`
+}
+
+func ftoa(f float64) string {
+	// small helper — avoids strconv import noise in the canned lines
+	switch f {
+	case 0:
+		return "0"
+	case 0.42:
+		return "0.42"
+	case 1:
+		return "1"
+	}
+	return "0.5"
+}
+
+func itoa(i int64) string {
+	if i == 0 {
+		return "0"
+	}
+	var b []byte
+	neg := i < 0
+	if neg {
+		i = -i
+	}
+	for i > 0 {
+		b = append([]byte{byte('0' + i%10)}, b...)
+		i /= 10
+	}
+	if neg {
+		return "-" + string(b)
+	}
+	return string(b)
+}
+
+// TestManualRunReportsParsedProgress is the headline: a manual run driven by a fake restic that emits
+// --json status lines must make OffboxProgressSnapshot report the parsed percentage, byte counts and
+// the app currently being pushed.
+func TestManualRunReportsParsedProgress(t *testing.T) {
+	e := newSharesOffboxEnv(t, "immich")
+	if err := e.sett.SetSMBEnabled(false); err != nil { // keep this test to the app leg only
+		t.Fatal(err)
+	}
+
+	var seen []OffboxProgress
+	var sawJSONFlag bool
+	e.m.SetOffboxStreamRunner(func(ctx context.Context, env []string, onLine func(string), args ...string) ([]byte, error) {
+		for _, a := range args {
+			if a == "--json" {
+				sawJSONFlag = true
+			}
+		}
+		// Emit a scan phase (total not yet known), then real progress, sampling the published
+		// snapshot after each line exactly as the polling page would.
+		onLine(jsonStatus(0, 0, 0))
+		seen = append(seen, e.m.OffboxProgressSnapshot())
+		onLine(`{"message_type":"verbose_status","action":"unchanged"}`) // must be ignored, not fatal
+		onLine(jsonStatus(0.42, 4200, 10000))
+		seen = append(seen, e.m.OffboxProgressSnapshot())
+		return []byte(`{"message_type":"summary","snapshot_id":"abc"}`), nil
+	})
+	// The non-streaming seam still serves every other restic call (cat config, forget, snapshots…).
+	e.m.SetOffboxRunner(func(ctx context.Context, env []string, args ...string) ([]byte, error) {
+		if contains(args, "snapshots") {
+			return []byte(`[]`), nil
+		}
+		return []byte(""), nil
+	})
+
+	if err := e.m.RunOffboxBackupWithProgress(context.Background()); err != nil {
+		t.Fatalf("manual run: %v", err)
+	}
+
+	if !sawJSONFlag {
+		t.Fatal("the manual backup leg did not pass --json to restic — nothing could ever be parsed")
+	}
+	if len(seen) != 2 {
+		t.Fatalf("expected 2 sampled snapshots, got %d", len(seen))
+	}
+
+	// While restic is still scanning, total is unknown: report 0 rather than inventing a percentage.
+	if seen[0].TotalBytes != 0 || seen[0].Percent != 0 {
+		t.Errorf("scan phase: got %+v, want zeroed counters", seen[0])
+	}
+	if seen[0].CurrentApp != "immich" {
+		t.Errorf("scan phase: current_app = %q, want %q", seen[0].CurrentApp, "immich")
+	}
+
+	got := seen[1]
+	if got.Percent != 42 {
+		t.Errorf("percent = %v, want 42", got.Percent)
+	}
+	if got.BytesDone != 4200 || got.TotalBytes != 10000 {
+		t.Errorf("bytes = %d/%d, want 4200/10000", got.BytesDone, got.TotalBytes)
+	}
+	if got.CurrentApp != "immich" {
+		t.Errorf("current_app = %q, want %q", got.CurrentApp, "immich")
+	}
+	if got.TotalHuman == "" || got.DoneHuman == "" {
+		t.Errorf("humanized byte strings not populated: %+v", got)
+	}
+	if !got.Active {
+		t.Error("progress reported inactive during a manual run")
+	}
+
+	// After the run the sink must be torn down, or the page would keep rendering a stale bar.
+	if after := e.m.OffboxProgressSnapshot(); after.Active || after.Percent != 0 {
+		t.Errorf("progress still published after the run: %+v", after)
+	}
+}
+
+// TestNightlyRunStaysSilent pins the scope decision: the scheduled run must neither install the sink
+// nor pass --json, so its output format (and everything that greps it) is untouched.
+func TestNightlyRunStaysSilent(t *testing.T) {
+	e := newSharesOffboxEnv(t, "immich")
+	if err := e.sett.SetSMBEnabled(false); err != nil {
+		t.Fatal(err)
+	}
+	e.m.SetOffboxStreamRunner(func(ctx context.Context, env []string, onLine func(string), args ...string) ([]byte, error) {
+		t.Fatal("the nightly run used the streaming runner — progress must be manual-only")
+		return nil, nil
+	})
+	var backupArgs []string
+	e.m.SetOffboxRunner(func(ctx context.Context, env []string, args ...string) ([]byte, error) {
+		if contains(args, "backup") {
+			backupArgs = append([]string{}, args...)
+		}
+		if contains(args, "snapshots") {
+			return []byte(`[]`), nil
+		}
+		return []byte(""), nil
+	})
+
+	if err := e.m.RunOffboxBackup(context.Background()); err != nil {
+		t.Fatalf("nightly run: %v", err)
+	}
+	if backupArgs == nil {
+		t.Fatal("no backup call was made")
+	}
+	if contains(backupArgs, "--json") {
+		t.Errorf("the nightly run passed --json: %v", backupArgs)
+	}
+	if p := e.m.OffboxProgressSnapshot(); p.Active {
+		t.Errorf("the nightly run published progress: %+v", p)
+	}
+}
+
+// TestParseResticStatusIgnoresNonStatus covers the lines restic actually interleaves with status
+// output. An unknown message_type must be ignored, never mistaken for progress — restic adds new
+// types between versions.
+func TestParseResticStatusIgnoresNonStatus(t *testing.T) {
+	for _, line := range []string{
+		`{"message_type":"summary","total_bytes_processed":123}`,
+		`{"message_type":"verbose_status","action":"new"}`,
+		`{"message_type":"error","error":{"message":"boom"}}`,
+		`not json at all`,
+		``,
+		`{}`,
+	} {
+		if _, _, _, ok := parseResticStatus(line); ok {
+			t.Errorf("parsed a non-status line as progress: %q", line)
+		}
+	}
+	pct, done, total, ok := parseResticStatus(jsonStatus(0.42, 4200, 10000))
+	if !ok {
+		t.Fatal("a real status line was not parsed")
+	}
+	if pct != 42 || done != 4200 || total != 10000 {
+		t.Errorf("got %v %d %d, want 42 4200 10000", pct, done, total)
+	}
+}
+
+// TestParseResticStatusClampsPercent — restic has been seen to report percent_done slightly above 1
+// near completion. A bar wider than its track is a visible bug.
+func TestParseResticStatusClampsPercent(t *testing.T) {
+	pct, _, _, ok := parseResticStatus(`{"message_type":"status","percent_done":1.04}`)
+	if !ok || pct != 100 {
+		t.Errorf("percent = %v (ok=%v), want clamped to 100", pct, ok)
+	}
+	pct, _, _, ok = parseResticStatus(`{"message_type":"status","percent_done":-0.2}`)
+	if !ok || pct != 0 {
+		t.Errorf("percent = %v (ok=%v), want clamped to 0", pct, ok)
+	}
+}
+
+// TestLineTailKeepsOnlyTheTail — the --json stream of a large backup is megabytes of status spam, and
+// every caller uses this output for error diagnosis and lock-pattern matching. Buffering all of it
+// would be a memory leak proportional to backup size.
+func TestLineTailKeepsOnlyTheTail(t *testing.T) {
+	var tl lineTail
+	for i := 0; i < 500; i++ {
+		tl.add("line-" + itoa(int64(i)))
+	}
+	out := string(tl.bytes())
+	if strings.Contains(out, "line-0\n") {
+		t.Error("the oldest line survived — the tail is unbounded")
+	}
+	if !strings.Contains(out, "line-499") {
+		t.Error("the newest line was dropped")
+	}
+	if n := strings.Count(out, "\n"); n > 64 {
+		t.Errorf("tail kept %d lines, want a small bounded number", n)
+	}
+}