felhom-controller/controller/internal/web/restore_inflight_guard_test.go

package web

import (
	"net/http/httptest"
	"net/url"
	"strings"
	"testing"

	"gitea.dooplex.hu/admin/felhom-controller/internal/settings"
)

// R-351 — THE SECOND PRESS. Part 3 asked whether a second press really starts a second run or is
// refused somewhere deeper. It is NOT refused: it starts a second run, and the customer is told so.
//
// WHY THE EXISTING GUARD DOES NOT CATCH IT. Every restore handler gates on `backupMgr.IsRunning()`
// (offbox_handlers.go:355/436/524/557/583, handlers.go:1404/1463). That reads `m.running`, the
// CONCURRENCY single-flight, which is acquired INSIDE the restore function on the background
// goroutine (offbox_reconstitute.go:180, offbox_restore.go:393) — not by the handler. So between the
// handler's check and the goroutine's acquire there is a window in which `IsRunning()` is false while
// a restore is unmistakably in flight. `restoreOpInFlight` and the whole wizard already read the
// other flag (`RestoreStatus().Running`, set synchronously by BeginRestoreOp) for exactly this
// reason — see the long note on restoreOpInFlight. The handlers were never moved over.
//
// THE FIXTURE IS THE LIVE STATE, NOT AN INVENTED ONE: display flag set, concurrency flag NOT held.
// That is precisely what the box looks like for the entire duration of an off-box restore.
//
// This test pins the CONSEQUENCE (does a second run start?), not the mechanism (which flag is read),
// per CLAUDE.md's preference. Mutating the new guard back to `IsRunning()` must make it fail.
func TestRestoreHandlers_SecondPressDoesNotStartASecondRun(t *testing.T) {
	const firstApp = "alpha"
	const secondApp = "beta"

	s, sett, m := newOffboxWebServer(t)
	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)
	}
	if err := m.WriteOffboxSecrets("PRIVATE-KEY-MATERIAL", "nas.local ssh-ed25519 AAAAhostkey"); err != nil {
		t.Fatal(err)
	}
	if !m.OffboxConfigured() {
		t.Fatal("fixture: the target must be configured, or the handler exits earlier and proves nothing")
	}

	// A restore is in flight, exactly as the live box has it.
	m.BeginRestoreOp("offbox-restore", firstApp)

	// The fixture must reproduce the GAP, or this test is vacuous: the display flag says running,
	// the concurrency flag — the one every handler reads — says it is not.
	if !m.RestoreStatus().Running {
		t.Fatal("fixture: the display flag must report a running op")
	}
	if m.IsRunning() {
		t.Fatal("fixture: the concurrency flag must NOT be held — that gap IS the defect under test")
	}

	post := func(path string, form url.Values) *httptest.ResponseRecorder {
		r := httptest.NewRequest("POST", path, strings.NewReader(form.Encode()))
		r.Header.Set("Content-Type", "application/x-www-form-urlencoded")
		w := httptest.NewRecorder()
		switch path {
		case "/backup/offbox/reconstitute":
			s.offboxReconstituteHandler(w, r)
		case "/backup/offbox/place":
			s.offboxPlaceHandler(w, r)
		default:
			t.Fatalf("unrouted path %q", path)
		}
		return w
	}

	for _, tc := range []struct {
		name string
		path string
		form url.Values
	}{
		{"reconstitute", "/backup/offbox/reconstitute", url.Values{"app": {secondApp}, "confirm": {"1"}}},
		{"place", "/backup/offbox/place", url.Values{"app": {secondApp}}},
	} {
		t.Run(tc.name, func(t *testing.T) {
			w := post(tc.path, tc.form)
			if w.Code != 302 {
				t.Fatalf("the handler redirects; got %d", w.Code)
			}
			loc := w.Header().Get("Location")

			// CONSEQUENCE 1 — the customer must not be told a second restore started.
			// "elind" is the ASCII stem shared by every „elindult" flash; matching it avoids putting
			// accented bytes through a comparison (strict rule 7).
			if strings.Contains(loc, "elind") {
				t.Errorf("a second press while a restore runs must NOT report a started restore. Location: %q", loc)
			}

			// CONSEQUENCE 2 — the in-flight op must still be the FIRST one. If the handler ran,
			// BeginRestoreOp overwrote the op name and stack, so the first restore's identity is
			// gone from the status the banner reads.
			st := m.RestoreStatus()
			if st.Op != "offbox-restore" || st.Stack != firstApp {
				t.Errorf("the first restore's identity was overwritten by the second press: op=%q stack=%q (want offbox-restore/%s)",
					st.Op, st.Stack, firstApp)
			}

			// CONSEQUENCE 3 — a refusal must name a route the person can act on, not just a reason.
			// The wizard path is the route; it is where the live status is shown.
			if !strings.Contains(loc, "/backups/restore") {
				t.Errorf("the refusal must route somewhere actionable; got %q", loc)
			}

			// Restore the fixture for the next subtest — a handler that (today) ran will have
			// clobbered it.
			m.BeginRestoreOp("offbox-restore", firstApp)
		})
	}
}