agent v0.105.0 — R-88 Part 2: /backup/due gains age_state

newestArchiveOn's (time.Time, bool) signature could not express the 'unknown' its own doc comment promised: a read error and a genuine not-found both returned (zero,false), so /backup/due emitted a POSITIVE 'no successful backup recorded yet' with a nil age, and the controller fired its window-gate valve on an unreadable storage. Three states now: known / absent / unknown, carried as a STRING enum so the zero value unambiguously means 'legacy agent' rather than masquerading as an answer. Fail-safe direction unchanged — unknown is still DUE; only the window-gate bypass narrows to ABSENT. A service with NO lister deliberately stays ABSENT: calling it unknown would stop a genuinely new box on a pre-R-84 build from ever backing up outside its window. An unparseable timestamp becomes unknown — a backup happened, we cannot date it.
2026-07-27 18:00:56 +02:00
parent 5bca7bfc9a
commit 1c2664b0c1
3 changed files with 282 additions and 12 deletions
@@ -0,0 +1,160 @@
+package localapi
+
+import (
+	"context"
+	"errors"
+	"io"
+	"log/slog"
+	"testing"
+	"time"
+
+	"gitea.dooplex.hu/admin/felhom-agent/internal/hub"
+)
+
+// R-88 Part 2 — the agent gains a third state.
+//
+// `newestArchiveOn` promised in its own doc comment that "errors degrade to unknown, never to
+// no-backup", while its `(time.Time, bool)` return made that impossible: an error and a genuine
+// not-found both produced `(zero, false)`, so `/backup/due` answered a POSITIVE
+// "no successful backup recorded yet" with a nil age. The controller read that as "never backed up"
+// and fired its window-gate safety valve, quiescing customer app stacks outside the backup window.
+//
+// These tests assert the WIRE, because the wire is the contract another component reads.
+
+// listerBackups is a fakeBackups that also implements BackupArchiveLister, with a controllable outcome.
+type listerBackups struct {
+	fakeBackups
+	t     time.Time
+	found bool
+	err   error
+}
+
+func (l *listerBackups) NewestArchiveTime(context.Context, int) (time.Time, bool, error) {
+	return l.t, l.found, l.err
+}
+
+// dueWithLister builds a server whose single tier's service is the given lister, and returns the
+// /backup/due response.
+// NOTE: the server must receive `lb` ITSELF, not its embedded fakeBackups — the tier's Service is
+// type-asserted to BackupArchiveLister, and the embedded value does not satisfy it. Passing the
+// inner struct silently routes every case to archiveUnknown, which looks like a code bug and is not.
+func dueWithLister(t *testing.T, lb *listerBackups, store *fakeStore) BackupDueResponse {
+	t.Helper()
+	srv, err := NewServer(Options{
+		ListenAddr:    "127.0.0.1:0",
+		Guests:        &fakeGuests{},
+		Backups:       lb,
+		Store:         store,
+		Storage:       fakeStorage{},
+		Tokens:        staticTokens{"A": 8200, "B": 9300},
+		BackupCadence: 24 * time.Hour,
+		Logger:        slog.New(slog.NewTextHandler(io.Discard, nil)),
+	})
+	if err != nil {
+		t.Fatalf("NewServer: %v", err)
+	}
+	srv.now = func() time.Time { return testNow }
+	return dueOf(t, srv.Handler())
+}
+
+// ── SCENARIO A (agent half) — an unreadable storage is UNKNOWN, not "never" ──────────────────
+//
+// COMPANION RED-PROOF (observed): make newestArchiveOn return archiveAbsent on the error path (the
+// pre-fix collapse) and this fails with
+//
+//	"an unreadable storage must report age_state=unknown, got \"absent\" — that is a POSITIVE claim
+//	 of 'never backed up' built out of two absences"
+//
+// Restored.
+func TestAgeState_UnreadableStorageIsUnknown(t *testing.T) {
+	lb := &listerBackups{err: errors.New("proxmox: GET storage content: 500 connection refused")}
+	got := dueWithLister(t, lb, &fakeStore{}) // empty store = cold in-memory record
+
+	if got.AgeState != AgeStateUnknown {
+		t.Fatalf("an unreadable storage must report age_state=%q, got %q — that is a POSITIVE claim of "+
+			"'never backed up' built out of two absences", AgeStateUnknown, got.AgeState)
+	}
+	if !got.Due {
+		t.Fatal("FAIL-SAFE DIRECTION: unknown must still be DUE — an unreadable storage must never suppress a backup")
+	}
+	if got.AgeSecs != nil {
+		t.Fatalf("an unknown age must not invent a number; got %d", *got.AgeSecs)
+	}
+}
+
+// ── SCENARIO B (agent half) — a genuine first-ever backup is ABSENT ──────────────────────────
+//
+// B is what makes A safe: an implementation that reported everything as "unknown" would pass A and
+// silently starve a brand-new box, because the controller only fires the first-backup valve on ABSENT.
+//
+// COMPANION RED-PROOF (observed): make newestArchiveOn return archiveUnknown when !found and this
+// fails with
+//
+//	"a genuine never-backed-up tier must report age_state=\"absent\", got \"unknown\" — the
+//	 controller only licenses a first backup outside the window on ABSENT"
+//
+// Restored.
+func TestAgeState_GenuinelyNeverIsAbsent(t *testing.T) {
+	lb := &listerBackups{found: false} // read SUCCEEDED, nothing there
+	got := dueWithLister(t, lb, &fakeStore{})
+
+	if got.AgeState != AgeStateAbsent {
+		t.Fatalf("a genuine never-backed-up tier must report age_state=%q, got %q — the controller only "+
+			"licenses a first backup outside the window on ABSENT", AgeStateAbsent, got.AgeState)
+	}
+	if !got.Due {
+		t.Fatal("a never-backed-up tier must be due")
+	}
+}
+
+// A real archive → known, with a real age.
+func TestAgeState_FoundIsKnown(t *testing.T) {
+	lb := &listerBackups{t: testNow.Add(-2 * time.Hour), found: true}
+	got := dueWithLister(t, lb, &fakeStore{})
+
+	if got.AgeState != AgeStateKnown {
+		t.Fatalf("a readable archive must report age_state=%q, got %q", AgeStateKnown, got.AgeState)
+	}
+	if got.AgeSecs == nil || *got.AgeSecs < 7100 || *got.AgeSecs > 7300 {
+		t.Fatalf("expected ~7200s age, got %v", got.AgeSecs)
+	}
+	if got.Due {
+		t.Fatal("2h old against a 24h cadence is not due")
+	}
+}
+
+// An unparseable in-memory timestamp is UNKNOWN too — a backup DID happen, we just cannot date it.
+// Reporting "absent" there would be the same false-positive claim in a different costume.
+func TestAgeState_UnparseableTimestampIsUnknown(t *testing.T) {
+	st := &fakeStore{backups: []hub.Backup{{VMID: 8200, Success: true, StartedAt: "not-a-timestamp"}}}
+	lb := &listerBackups{err: errors.New("storage unreadable")}
+	got := dueWithLister(t, lb, st)
+
+	if got.AgeState != AgeStateUnknown {
+		t.Fatalf("an unparseable backup time means we cannot DATE a backup that exists — want %q, got %q",
+			AgeStateUnknown, got.AgeState)
+	}
+	if !got.Due {
+		t.Fatal("still due — fail safe toward taking a backup")
+	}
+}
+
+// ── SCENARIO D (agent half) — additive on the wire ───────────────────────────────────────────
+//
+// An OLD controller decodes into a struct without `age_state` and ignores it. What it MUST still see
+// unchanged is every pre-existing field.
+func TestAgeState_IsAdditive_PreExistingFieldsUnchanged(t *testing.T) {
+	lb := &listerBackups{t: testNow.Add(-48 * time.Hour), found: true}
+	got := dueWithLister(t, lb, &fakeStore{})
+
+	if !got.Due || got.Reason != "older than cadence" {
+		t.Fatalf("pre-existing due/reason semantics changed: due=%v reason=%q", got.Due, got.Reason)
+	}
+	if got.AgeSecs == nil {
+		t.Fatal("age_seconds must still be present for a known age")
+	}
+	// And the state rides alongside rather than replacing anything.
+	if got.AgeState != AgeStateKnown {
+		t.Fatalf("age_state should be %q, got %q", AgeStateKnown, got.AgeState)
+	}
+}