R-204 item 3: a restore says what it restored, and what it did not (v0.198.0)

mode=unit restores the recovery unit — the app's definition, configuration
and database dumps — and NOT the customer's own files: RestoreOffboxScratch
passes --include <unit path> and the userdata in the same snapshot is excluded
by it. The outcome was one sentence for both modes and named neither scope,
so on the last step of a disaster recovery the customer was told the app had
been restored after the thing they were looking for had not been.

restoreScratchOutcomeMsg states what came back, what did not, and the next
step that gets it. The wizard's intent card states its scope before the choice.
The full-restore size gate is untouched and pinned as unchanged; the default
stays unit, since all three wizard forms set mode explicitly.

controller/internal/web/offbox_handlers.go

@@ -345,15 +345,40 @@ func (s *Server) offboxRestoreHandler(w http.ResponseWriter, r *http.Request) {
 		// customer had no way to look at what they had just asked for. Resolve the real path and say
 		// it. Fall back to the vague wording only if the path can no longer be resolved.
 		where := s.backupMgr.OffsiteRestoreScratchPath(app)
-		msg := "A(z) " + app + " visszaállítva ellenőrző mappába a meghajtón (a meglévő adatok változatlanok)."
-		if where != "" {
-			msg = "A(z) " + app + " visszaállítva ellenőrző mappába: " + where + " (a meglévő adatok változatlanok)."
-		}
-		s.backupMgr.EndRestoreOp(true, msg)
+		s.backupMgr.EndRestoreOp(true, restoreScratchOutcomeMsg(app, where, full))
 	}()
 	offboxRedirectTo(w, r, restoreWizardPath(app), "A távoli visszaállítás elindult — az állapot itt frissül.", false)
 }
 
+// restoreScratchOutcomeMsg builds the OUTCOME flash for a completed scratch restore. Pure, so the
+// wording is unit-testable — this string is the customer's only evidence of WHAT they now have.
+//
+// R-204 item 3 (v0.198.0) — THE DEFECT IT CLOSES. The default restore (`mode=unit`) recovers the
+// recovery unit: the app's definition, its configuration and its database dumps. It does NOT recover
+// the customer's own files; `RestoreOffboxScratch` passes `--include <unit path>` and the userdata
+// paths that ARE in the same snapshot are excluded by it. The old message was one sentence for both
+// modes and named neither scope, so a customer on the last step of a disaster recovery was told
+// „visszaállítva" after the thing they were looking for had not been restored. A success message
+// that does not name its scope is a silent wrong answer, which is this project's most-repeated
+// failure shape.
+//
+// So the unit case states three things in order: what came back, what did NOT, and the next step
+// that gets it. The full case says the files came with it, because otherwise the absence of the
+// warning would be the only difference and an absence is not a statement.
+func restoreScratchOutcomeMsg(app, where string, full bool) string {
+	at := " ellenőrző mappába"
+	if where != "" {
+		at = " ellenőrző mappába: " + where
+	}
+	if full {
+		return "A(z) " + app + " teljes mentése visszaállítva" + at +
+			" — a saját fájljaiddal együtt. A meglévő adatok változatlanok."
+	}
+	return "A(z) " + app + " beállításai és adatbázisa visszaállítva" + at +
+		". A saját fájljaid (dokumentumok, képek, feltöltések) NEM kerültek vissza — ez az ellenőrző visszaállítás csak az alkalmazás beállításait és adatbázisát hozza vissza. " +
+		"Ha a fájljaidra van szükséged, indítsd el a „Teljes visszaállítás előkészítése” lépést ezen az oldalon. A meglévő adatok változatlanok."
+}
+
 // offboxReconstituteHandler is the TRUE offsite restore (R-43, v0.148.0): files overwritten to the
 // snapshot's version + that same snapshot's database replayed + the app restarted, with a safety
 // dump of the current database taken first.

controller/internal/web/offbox_restore_scope_test.go

@@ -0,0 +1,239 @@
+package web
+
+import (
+	"context"
+	"encoding/json"
+	"io"
+	"log"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"os"
+	"path/filepath"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+
+	"gitea.dooplex.hu/admin/felhom-controller/internal/backup"
+	"gitea.dooplex.hu/admin/felhom-controller/internal/config"
+	"gitea.dooplex.hu/admin/felhom-controller/internal/settings"
+)
+
+// R-204 item 3 — a restore must say WHAT IT RESTORED, and for the default mode, what it did not.
+//
+// THESE TESTS DRIVE THE REAL HANDLER, not the message helper (task §10: a test that reaches a helper
+// while the mutation lives in the handler cannot observe it). offboxRestoreHandler runs, the restic
+// exec is the only thing stubbed, and the assertion is on the flash the customer actually receives —
+// backupMgr.RestoreStatus().Last.Message, the same field the wizard renders.
+
+// scopeRunner is the restic exec seam. It answers the three calls a scratch restore makes
+// (snapshots / unlock / restore) and RECORDS the restore argv, so Scenario F can assert the
+// unit-vs-full distinction is still carried where it matters.
+type scopeRunner struct {
+	mu          sync.Mutex
+	unitPath    string
+	restoreArgs []string
+	sizeBytes   int64
+}
+
+func (sr *scopeRunner) run(_ context.Context, _ []string, args ...string) ([]byte, error) {
+	sr.mu.Lock()
+	defer sr.mu.Unlock()
+	joined := strings.Join(args, " ")
+	switch {
+	case strings.Contains(joined, " snapshots ") || strings.HasSuffix(joined, " snapshots"):
+		out, _ := json.Marshal([]map[string]any{{
+			"short_id": "abc1234",
+			"id":       "abc1234deadbeef",
+			"time":     time.Now().UTC().Format(time.RFC3339),
+			// The snapshot holds BOTH the recovery unit and the customer's userdata — which is the
+			// whole point: a unit restore leaves the second one behind.
+			"paths": []string{sr.unitPath, filepath.Dir(filepath.Dir(sr.unitPath)) + "/userdata/immich"},
+		}})
+		return out, nil
+	case strings.Contains(joined, " stats "):
+		out, _ := json.Marshal(map[string]any{"total_size": sr.sizeBytes})
+		return out, nil
+	case strings.Contains(joined, " restore "):
+		sr.restoreArgs = append([]string{}, args...)
+		return []byte("restored"), nil
+	}
+	return []byte(""), nil // unlock and anything else: a clean no-op
+}
+
+func (sr *scopeRunner) lastRestoreArgs() []string {
+	sr.mu.Lock()
+	defer sr.mu.Unlock()
+	return append([]string{}, sr.restoreArgs...)
+}
+
+// scopeServer wires a Server with a configured offbox manager whose restic exec is the stub above.
+// The drive is a real temp dir registered as schedulable, so the scratch path resolves for real.
+func scopeServer(t *testing.T) (*Server, *backup.Manager, *scopeRunner) {
+	t.Helper()
+	lg := log.New(io.Discard, "", 0)
+	dir := t.TempDir()
+	drive := filepath.Join(dir, "usb")
+	if err := os.MkdirAll(drive, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	cfg := &config.Config{}
+	cfg.Paths.DataDir = filepath.Join(dir, "data")
+	cfg.Paths.SystemDataPath = filepath.Join(dir, "sys")
+	cfg.Paths.StacksDir = filepath.Join(dir, "stacks")
+	cfg.Web.SessionSecret = "test-session-secret-abcdef"
+
+	sett, err := settings.Load(filepath.Join(dir, "settings.json"), lg)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := sett.AddStoragePath(settings.StoragePath{Path: drive, Label: "USB", Schedulable: true}); err != nil {
+		t.Fatal(err)
+	}
+	if err := sett.SetOffboxTarget(&settings.OffboxTarget{
+		Enabled: true, Host: "nas.local", Port: 22, User: "felhom", RepoPath: "/srv/repo",
+		Schedule: "daily", EscrowState: "escrowed",
+	}); err != nil {
+		t.Fatal(err)
+	}
+	m := backup.NewManager(cfg, sett, lg)
+	if err := m.WriteOffboxSecrets("PRIVATE-KEY-MATERIAL", "nas.local ssh-ed25519 AAAAhostkey"); err != nil {
+		t.Fatal(err)
+	}
+	if !m.OffboxConfigured() {
+		t.Fatal("offbox target not configured — the handler would refuse before reaching the outcome")
+	}
+	sr := &scopeRunner{
+		// The unit path as it appears INSIDE the snapshot: <nsRoot>/backups/primary/<app>.
+		unitPath:  strings.TrimSuffix(m.OffsiteRestoreScratchPath("immich"), "/backups/offsite-restore/immich") + "/backups/primary/immich",
+		sizeBytes: 4 << 20, // 4 MiB — comfortably inside the headroom of a temp dir
+	}
+	m.SetOffboxRunner(sr.run)
+
+	s := &Server{cfg: cfg, settings: sett, backupMgr: m, logger: lg, version: "test"}
+	s.loadTemplates()
+	return s, m, sr
+}
+
+// postRestore drives the REAL handler and waits for the async restore to finish.
+func postRestore(t *testing.T, s *Server, m *backup.Manager, form url.Values) backup.RestoreOpStatus {
+	t.Helper()
+	req := httptest.NewRequest(http.MethodPost, "/backup/offbox/restore", strings.NewReader(form.Encode()))
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	rr := httptest.NewRecorder()
+	s.offboxRestoreHandler(rr, req)
+
+	// Wait on a REAL completion marker (a finished Last with a FinishedAt), never a fixed sleep.
+	deadline := time.Now().Add(10 * time.Second)
+	for time.Now().Before(deadline) {
+		st := m.RestoreStatus()
+		if !st.Running && st.Last != nil && !st.Last.FinishedAt.IsZero() {
+			return st
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+	t.Fatalf("restore did not finish within the deadline (status=%+v)", m.RestoreStatus())
+	return backup.RestoreOpStatus{}
+}
+
+// SCENARIO E — the DEFAULT (unit) restore's outcome names what it did NOT restore, and the next step.
+//
+// RED-PROOF: delete the „NEM kerültek vissza" sentence from restoreScratchOutcomeMsg (or revert the
+// function to the single pre-R-204 sentence). The handler still succeeds and still flashes a
+// „visszaállítva" message — and this test fails, which is exactly the silence being closed.
+func TestOffboxRestore_UnitOutcomeNamesWhatItDidNotRestore(t *testing.T) {
+	s, m, _ := scopeServer(t)
+
+	st := postRestore(t, s, m, url.Values{"app": {"immich"}, "mode": {"unit"}})
+	if !st.Last.OK {
+		t.Fatalf("unit restore failed: %q", st.Last.Message)
+	}
+	msg := st.Last.Message
+
+	// It must name what CAME BACK…
+	for _, want := range []string{"immich", "be" + "állításai és adatbázisa visszaállítva"} {
+		if !strings.Contains(msg, want) {
+			t.Errorf("outcome does not state what was restored (missing %q): %q", want, msg)
+		}
+	}
+	// …and, the point of R-204 item 3, what did NOT.
+	if !strings.Contains(msg, "NEM kerültek vissza") {
+		t.Errorf("outcome does not state that the customer's own files were NOT restored: %q", msg)
+	}
+	if !strings.Contains(msg, "dokumentumok") {
+		t.Errorf("outcome does not name the files it left behind: %q", msg)
+	}
+	// …and the next step that actually gets them.
+	if !strings.Contains(msg, "Teljes vissza"+"állítás előkészítése") {
+		t.Errorf("outcome does not name the next step that returns the files: %q", msg)
+	}
+	// The scratch path is still named (the v0.147.0 4a guarantee must not regress).
+	if !strings.Contains(msg, m.OffsiteRestoreScratchPath("immich")) {
+		t.Errorf("outcome no longer names the folder it restored into: %q", msg)
+	}
+}
+
+// The DEFAULT is `unit` (mode absent) and it must produce the SAME scoped outcome — the wizard always
+// sets mode, but a mode-less POST must not fall into a message that overstates what it did.
+func TestOffboxRestore_DefaultModeGetsTheScopedOutcome(t *testing.T) {
+	s, m, _ := scopeServer(t)
+	st := postRestore(t, s, m, url.Values{"app": {"immich"}}) // no mode at all
+	if !strings.Contains(st.Last.Message, "NEM kerültek vissza") {
+		t.Fatalf("the DEFAULT restore did not state its scope: %q", st.Last.Message)
+	}
+}
+
+// SCENARIO F — the full restore is unchanged: still two-step and size-gated, and its outcome does NOT
+// carry the unit warning (a full restore did bring the files).
+func TestOffboxRestore_FullPathUnchanged(t *testing.T) {
+	s, m, sr := scopeServer(t)
+
+	// Step 1: mode=full WITHOUT confirm must NOT restore — it computes and redirects with the reveal.
+	req := httptest.NewRequest(http.MethodPost, "/backup/offbox/restore",
+		strings.NewReader(url.Values{"app": {"immich"}, "mode": {"full"}}.Encode()))
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	rr := httptest.NewRecorder()
+	s.offboxRestoreHandler(rr, req)
+	if rr.Code != http.StatusFound {
+		t.Fatalf("full step 1: want a redirect, got %d", rr.Code)
+	}
+	loc := rr.Header().Get("Location")
+	if !strings.Contains(loc, "full_prep=immich") || !strings.Contains(loc, "full_size=") {
+		t.Fatalf("full step 1 did not reveal the size gate: Location=%q", loc)
+	}
+	if len(sr.lastRestoreArgs()) != 0 {
+		t.Fatal("full step 1 ran a restore before the customer confirmed — the size gate is bypassed")
+	}
+
+	// Step 2: the revealed confirm executes, and the outcome says the files came with it.
+	st := postRestore(t, s, m, url.Values{"app": {"immich"}, "mode": {"full"}, "confirm": {"1"}})
+	if !st.Last.OK {
+		t.Fatalf("full restore failed: %q", st.Last.Message)
+	}
+	if strings.Contains(st.Last.Message, "NEM kerültek vissza") {
+		t.Fatalf("the FULL restore wrongly claims the files were left behind: %q", st.Last.Message)
+	}
+	if !strings.Contains(st.Last.Message, "saját fájljaiddal együtt") {
+		t.Fatalf("the full outcome does not state that the files came with it: %q", st.Last.Message)
+	}
+	// And the mechanism that makes the two modes differ is still carried: unit passes --include, full
+	// does not. Asserted on the REAL argv the manager built.
+	if args := sr.lastRestoreArgs(); strings.Contains(strings.Join(args, " "), "--include") {
+		t.Fatalf("a FULL restore must not restrict to the unit: %v", args)
+	}
+}
+
+// The unit restore's mechanism half: it DOES restrict to the unit path. Without this, Scenario E's
+// message could be true today and quietly become a lie if --include were dropped.
+func TestOffboxRestore_UnitRestrictsToTheUnitPath(t *testing.T) {
+	s, m, sr := scopeServer(t)
+	postRestore(t, s, m, url.Values{"app": {"immich"}, "mode": {"unit"}})
+	joined := strings.Join(sr.lastRestoreArgs(), " ")
+	if !strings.Contains(joined, "--include") {
+		t.Fatalf("a unit restore must restrict to the unit path, argv=%q", joined)
+	}
+	if !strings.Contains(joined, "/backups/primary/immich") {
+		t.Fatalf("a unit restore did not include the recovery unit path, argv=%q", joined)
+	}
+}

controller/internal/web/restore_wizard_test.go

@@ -192,7 +192,11 @@ func TestRestoreWizard_ThreeIntentCards(t *testing.T) {
 	// Each intent must state its CONSEQUENCE, not just its name.
 	for _, want := range []string{
 		"Ellenőrzés külön mappába",
-		"az élő adataid nem változnak",
+		"Az élő adataid nem változnak",
+		// R-204 item 3: intent 1's SCOPE is stated before the choice, not only in the outcome. A
+		// disaster-recovery customer picked this card expecting their documents; it does not return
+		// them. If this substring goes, the card is back to promising „a mentés tartalma".
+		"A saját fájljaidat (dokumentumok, képek, feltöltések) <strong>nem</strong> hozza vissza",
 		"Hiányzó fájlok visszahozása",
 		"törölt tartalom ettől nem jelenik meg újra",
 		"Teljes visszaállítás (fájlok + adatbázis)",

controller/internal/web/templates/backups_restore_wizard.html

@@ -74,8 +74,12 @@
      harmless first, irreversible-looking last. -->
 
 <div class="settings-card">
-    <h3>1. Ellenőrzés külön mappába</h3>
-    <p>A mentés tartalma egy külön ellenőrző mappába kerül — az élő adataid nem változnak.</p>
+    <h3>1. Ellenőrzés külön mappába (beállítások és adatbázis)</h3>
+    <!-- R-204 item 3: this card's scope is stated BEFORE the choice, not only in the outcome. It
+         restores the recovery unit only; the customer's own files stay in the backup. Saying „a
+         mentés tartalma" here was how a disaster-recovery customer chose the one intent that does
+         not return their documents. -->
+    <p>Az alkalmazás beállításait és adatbázisát hozza vissza egy külön ellenőrző mappába. A saját fájljaidat (dokumentumok, képek, feltöltések) <strong>nem</strong> hozza vissza — azokhoz a 3. pont teljes visszaállítása kell. Az élő adataid nem változnak.</p>
     <div class="form-actions">
         <form method="POST" action="/backup/offbox/restore">{{.CSRFField}}
             <input type="hidden" name="app" value="{{.App}}">