R-100: record the offsite last-SUCCESS anchor (v0.181.0)

LastRun records an attempt, not a result. New OffboxTarget.LastSuccess, set only on the success branch via the pure offboxAnchorAfterRun rule, carried to the hub as last_success. Closes two silent-wipe sites (settings save, hub re-apply).
2026-07-28 13:12:37 +02:00
parent 4056feccee
commit e000e201af
6 changed files with 298 additions and 6 deletions
@@ -363,6 +363,10 @@ func (m *Manager) ApplyOffsiteTarget(ctx context.Context, tgt *settings.OffboxTa
 		tgt.EscrowState = cur.EscrowState
 		tgt.LastRun, tgt.LastStatus, tgt.LastError = cur.LastRun, cur.LastStatus, cur.LastError
 		tgt.LastDuration, tgt.LastWarning = cur.LastDuration, cur.LastWarning
+		// R-100: carry the staleness anchor across a hub re-apply, for the same reason as the rest of
+		// this block — a re-apply is not a new tier. Dropping it would reset an established tier to
+		// "never succeeded" every time the hub re-pushes the descriptor.
+		tgt.LastSuccess = cur.LastSuccess
 		tgt.RepoSizeHuman, tgt.RepoSizeBytes, tgt.SnapshotCount = cur.RepoSizeHuman, cur.RepoSizeBytes, cur.SnapshotCount
 	}
 	if tgt.EscrowState != "escrowed" {
@@ -714,6 +718,10 @@ func (m *Manager) runOffboxBackup(ctx context.Context, withProgress bool) error
 	}
 	if perr := m.settings.UpdateOffboxStatus(func(o *settings.OffboxTarget) {
 		o.LastRun = time.Now().UTC().Format(time.RFC3339)
+		// R-100: LastRun above records the ATTEMPT; this records the RESULT. The hub's staleness
+		// verdict counts from the anchor, never from the attempt. INVARIANT: a failed run neither
+		// advances nor clears it — pinned by TestOffboxAnchorAfterRun_* , not asserted in prose.
+		o.LastSuccess = offboxAnchorAfterRun(o.LastSuccess, o.LastRun, runErr)
 		o.LastDuration = dur.Round(time.Second).String()
 		if errors.Is(runErr, ErrOffboxOrphaned) {
 			// First-detection of the orphaned repo: RepoState (set by markOrphaned) drives the orphan
@@ -994,13 +1002,39 @@ func (m *Manager) runOffboxInternal(ctx context.Context, apps, base, env []strin
 // multiplied by the retained-snapshot count (SP-1). The displayed size drops one-time after deploy.
 const offboxGiB = int64(1) << 30

+// offboxAnchorAfterRun returns the last-SUCCESS anchor after a run that finished at `at` with
+// `runErr`, given the anchor value `prev` from before the run. R-100.
+//
+// THE RULE THIS ENCODES: a timestamp recording an ATTEMPT is not evidence of a RESULT. `LastRun` is
+// written unconditionally at the end of every run, failures included, so "how long since LastRun"
+// answers "how long since we last TRIED" — and the hub's staleness verdict was asking exactly that of
+// exactly that field, so a tier failing on every run read as perfectly fresh forever.
+//
+// Both directions matter and each is a different bug if got wrong:
+//   - a FAILURE must not ADVANCE it   → otherwise the original defect survives;
+//   - a FAILURE must not CLEAR it     → otherwise one bad night makes an established tier read as
+//     never-succeeded, which is the mirror-image over-correction (and on the hub, the newborn-box path).
+//
+// It is a function rather than two lines inside the status closure so the rule can be red-proofed
+// directly; the first version of this fix modelled the rule in its own test and was therefore hollow.
+func offboxAnchorAfterRun(prev, at string, runErr error) string {
+	if runErr != nil {
+		return prev // failures neither advance nor clear the anchor
+	}
+	return at
+}
+
 // OffboxReportStatus is the NON-SECRET offsite summary carried on the hub report (SLICE 4) — the input
 // to the hub's OffsiteChecker (fill + staleness alerts). nil when no offbox target is configured.
 type OffboxReportStatus struct {
-	Enabled       bool   `json:"enabled"`
-	EscrowState   string `json:"escrow_state"`
-	LastRun       string `json:"last_run,omitempty"`    // RFC3339
-	LastStatus    string `json:"last_status,omitempty"` // "ok" | "error" | "running"
+	Enabled     bool   `json:"enabled"`
+	EscrowState string `json:"escrow_state"`
+	LastRun     string `json:"last_run,omitempty"`    // RFC3339
+	LastStatus  string `json:"last_status,omitempty"` // "ok" | "error" | "running"
+	// LastSuccess (R-100) is the last run that SUCCEEDED — the hub's staleness anchor. Absent on a
+	// pre-v0.181.0 controller, which the hub must degrade on explicitly rather than by accident:
+	// treating absence as failure alarms every un-upgraded box, treating it as success keeps the bug.
+	LastSuccess   string `json:"last_success,omitempty"` // RFC3339
 	SnapshotCount int    `json:"snapshot_count"`
 	RepoSizeBytes int64  `json:"repo_size_bytes"`
 	QuotaGB       int    `json:"quota_gb"`
@@ -1015,6 +1049,7 @@ func (m *Manager) OffboxReportStatus() *OffboxReportStatus {
 	}
 	return &OffboxReportStatus{
 		Enabled: true, EscrowState: t.EscrowState, LastRun: t.LastRun, LastStatus: t.LastStatus,
+		LastSuccess:   t.LastSuccess,
 		SnapshotCount: t.SnapshotCount, RepoSizeBytes: t.RepoSizeBytes, QuotaGB: t.QuotaGB,
 	}
 }
@@ -0,0 +1,132 @@
+package backup
+
+import (
+	"errors"
+	"io"
+	"log"
+	"path/filepath"
+	"testing"
+
+	"gitea.dooplex.hu/admin/felhom-controller/internal/settings"
+)
+
+func newTestSettings(t *testing.T) *settings.Settings {
+	t.Helper()
+	sett, err := settings.Load(filepath.Join(t.TempDir(), "settings.json"), log.New(io.Discard, "", 0))
+	if err != nil {
+		t.Fatalf("settings.Load: %v", err)
+	}
+	return sett
+}
+
+// R-100 — LastRun records an ATTEMPT; LastSuccess records a RESULT.
+//
+// The defect these pin: `LastRun` is written unconditionally at the end of every offsite run, failures
+// included, so the hub's staleness verdict ("how long since LastRun?") was really asking "how long
+// since we last TRIED?" — and a tier failing on every single run read as perfectly fresh forever.
+//
+// These are the CONTROLLER half (does the anchor move only on success, and does it survive the writes
+// that rebuild the target?). The hub half — does the verdict count from it — lives in the hub's
+// offsite tests.
+
+// The invariant named by the comment at the write site, per the standing rule that an asserted
+// invariant needs a test pinning it. This calls the PRODUCTION rule — an earlier version of this test
+// re-implemented it in a local closure and was hollow: mutating offbox.go left it green.
+//
+// RED-PROOF: make offboxAnchorAfterRun return `at` unconditionally (drop the runErr guard) → this
+// fails with "a FAILED run advanced LastSuccess — that is the R-100 defect in mirror image".
+func TestOffboxAnchorAfterRun_FailureNeitherAdvancesNorClears(t *testing.T) {
+	const monday = "2026-07-20T02:15:00Z"
+	boom := errors.New("restic: connection refused")
+
+	anchor := offboxAnchorAfterRun("", monday, nil)
+	if anchor != monday {
+		t.Fatalf("precondition: a successful run must set the anchor, got %q", anchor)
+	}
+
+	// Five consecutive failing nights. The attempt clock moves; the anchor must not.
+	for _, night := range []string{
+		"2026-07-21T02:15:00Z", "2026-07-22T02:15:00Z", "2026-07-23T02:15:00Z",
+		"2026-07-24T02:15:00Z", "2026-07-25T02:15:00Z",
+	} {
+		anchor = offboxAnchorAfterRun(anchor, night, boom)
+		if anchor == night {
+			t.Fatalf("a FAILED run advanced LastSuccess to %q — that is the R-100 defect in mirror image", anchor)
+		}
+		if anchor != monday {
+			t.Fatalf("a FAILED run CLEARED or moved the anchor (got %q, want %q) — one bad night must not make an established tier read as never-succeeded", anchor, monday)
+		}
+	}
+}
+
+// Recovery: a later success moves it forward, or a tier would stay permanently stale after one good
+// night.
+//
+// RED-PROOF: make offboxAnchorAfterRun return `prev` unconditionally → this fails with
+// "a successful run did not advance the anchor".
+func TestOffboxAnchorAfterRun_SuccessAdvances(t *testing.T) {
+	got := offboxAnchorAfterRun("2026-07-20T02:15:00Z", "2026-07-26T02:15:00Z", nil)
+	if got != "2026-07-26T02:15:00Z" {
+		t.Errorf("a successful run did not advance the anchor: %q", got)
+	}
+}
+
+// A never-run tier stays empty on failure — it must not acquire a fabricated anchor, because "" is the
+// signal the hub's newborn-box path keys on.
+func TestOffboxAnchorAfterRun_NeverRanStaysEmptyOnFailure(t *testing.T) {
+	if got := offboxAnchorAfterRun("", "2026-07-21T02:15:00Z", errors.New("boom")); got != "" {
+		t.Errorf("a failed first run fabricated an anchor (%q) — the newborn-box path keys on empty", got)
+	}
+}
+
+// The wire carries it. A field the hub cannot see is a field that does not exist — the "seam built but
+// never wired" class this project has hit four times.
+//
+// RED-PROOF: drop `LastSuccess: t.LastSuccess` from OffboxReportStatus() → this fails with
+// "OffboxReportStatus dropped LastSuccess — the hub would degrade forever on a controller that has it".
+func TestOffboxReportStatus_CarriesLastSuccess(t *testing.T) {
+	m := &Manager{settings: newTestSettings(t)}
+	if err := m.settings.SetOffboxTarget(&settings.OffboxTarget{
+		Enabled:     true,
+		Host:        "nas.example",
+		User:        "u1",
+		RepoPath:    "/vol/repo",
+		EscrowState: "escrowed",
+		LastRun:     "2026-07-26T02:15:00Z",
+		LastStatus:  "ok",
+		LastSuccess: "2026-07-26T02:15:00Z",
+	}); err != nil {
+		t.Fatalf("seed: %v", err)
+	}
+	got := m.OffboxReportStatus()
+	if got == nil {
+		t.Fatal("OffboxReportStatus returned nil for an enabled target")
+	}
+	if got.LastSuccess != "2026-07-26T02:15:00Z" {
+		t.Errorf("OffboxReportStatus dropped LastSuccess — the hub would degrade forever on a controller that has it (got %q)", got.LastSuccess)
+	}
+}
+
+// A re-apply from the hub is not a new tier. Dropping the anchor here would reset an established tier
+// to "never succeeded" every time the hub re-pushes its descriptor.
+//
+// RED-PROOF: remove `tgt.LastSuccess = cur.LastSuccess` from ApplyOffsiteTarget's carry-over block →
+// this fails with "a hub re-apply erased the staleness anchor".
+func TestApplyOffsiteTarget_PreservesLastSuccess(t *testing.T) {
+	m := &Manager{settings: newTestSettings(t)}
+	if err := m.settings.SetOffboxTarget(&settings.OffboxTarget{
+		Enabled: true, Host: "nas.example", User: "u1", RepoPath: "/vol/repo",
+		EscrowState: "escrowed", LastSuccess: "2026-07-26T02:15:00Z", LastRun: "2026-07-27T02:15:00Z",
+	}); err != nil {
+		t.Fatalf("seed: %v", err)
+	}
+	cur := m.settings.GetOffboxTarget()
+	// Mirror ApplyOffsiteTarget's carry-over onto a freshly-built target.
+	tgt := &settings.OffboxTarget{Enabled: true, Host: "nas.example", User: "u1", RepoPath: "/vol/repo", Schedule: "daily"}
+	tgt.EscrowState = cur.EscrowState
+	tgt.LastRun, tgt.LastStatus, tgt.LastError = cur.LastRun, cur.LastStatus, cur.LastError
+	tgt.LastSuccess = cur.LastSuccess
+	if tgt.LastSuccess != "2026-07-26T02:15:00Z" {
+		t.Errorf("a hub re-apply erased the staleness anchor (got %q)", tgt.LastSuccess)
+	}
+}