restic: let a restore point taken by hand go only after a night recorded good, say a missing hand= as a file to push, and say what a failed thinning already let go (novox/hq issue 453)

modules/restic/README.md

@@ -0,0 +1,23 @@
+# restic
+
+The machine's backups as the `node-backup` seat (novox/hq ADR 0214, to-be 43, ADR 0233): one restic
+repository on the machine, one restore point per module a night, thinned by the machine's settings.
+Against mistakes, not disasters: nothing leaves the machine.
+
+## How restore points are kept
+
+- **A night's restore point** is tagged with its module alone and thinned by `keep-daily`,
+  `keep-weekly` and `keep-monthly` (14, 8 and 6 by default).
+- **A restore point taken by hand** (`node-backup.now`, asked by a person, or by the mesh before a
+  module moves) is tagged `hand` as well. The thinning keeps that tag (`--keep-tag hand`), so the
+  next night never takes it (novox/hq issue 453). It is let go, by its id, only when both hold:
+  it is older than `keep-hand-taken-days` (7 by default), and a later night of its module was
+  recorded as good. A partial night, where restic wrote a restore point but could not read every
+  file, does not count.
+- **A module that left the machine** gets no later night, so its restore points taken by hand are
+  kept for good. That is a choice: deleting a module's last restore point is a person's act, and
+  until then it costs only space.
+
+`node-backup.backed-up` lists each module's restore points taken by hand under `handTaken`: when
+each was taken, the time it is kept at least until, whether it still waits for a later good night,
+and a sentence saying so.

modules/restic/cmd/restic-backups/backups.go

@@ -379,6 +379,9 @@ func (b *Backups) backUp(ctx context.Context, only string, hand bool) (map[strin
 	for _, d := range chosen {
 		outcome[d.Module] = b.one(ctx, d, hand)
 		b.record(d.Module, outcome[d.Module])
+		if n := outcome[d.Module]; !hand && n.OK && n.Snapshot != "" {
+			b.recordGoodNight(d.Module, n.Snapshot)
+		}
 	}
 	// A retention that does not read thins nothing: keeping too much costs space, thinning by a value
 	// nobody meant loses restore points (novox/hq issue 416).
@@ -388,8 +391,13 @@ func (b *Backups) backUp(ctx context.Context, only string, hand bool) (map[strin
 		thinned.Refused = err.Error()
 		b.Say("the retention does not read, so nothing is thinned and every restore point is kept: %v", err)
 	} else if letGo, err := b.thin(ctx, keep); err != nil {
-		thinned.Kept, thinned.Error = &keep, err.Error()
-		b.Say("thinning the restore points failed, and every one is kept: %v", err)
+		thinned.Kept, thinned.Error, thinned.LetGo = &keep, err.Error(), letGo
+		if len(letGo) > 0 {
+			b.Say("thinning the restore points failed after letting go %s, taken by hand; every other one is kept: %v",
+				strings.Join(letGo, ", "), err)
+		} else {
+			b.Say("thinning the restore points failed, and every one is kept: %v", err)
+		}
 	} else {
 		thinned.Kept, thinned.OK, thinned.LetGo = &keep, true, letGo
 	}
@@ -400,7 +408,7 @@ func (b *Backups) backUp(ctx context.Context, only string, hand bool) (map[strin
 
 // thin forgets the restore points taken by hand that are past their days and have a later night of their
 // module, by id, then thins the rest by the counts, keeping every one still tagged as taken by hand (novox/hq
-// issue 453). It answers the short ids it let go. A list that does not read thins nothing: a restore point
+// issue 453). It answers the short ids it let go — on an error of the counts' thinning too, for those are gone. A list that does not read thins nothing: a restore point
 // taken by hand is never lost to a list nobody could read.
 func (b *Backups) thin(ctx context.Context, keep Retention) ([]string, error) {
 	snaps, err := b.Snapshots(ctx, "")
@@ -408,7 +416,7 @@ func (b *Backups) thin(ctx context.Context, keep Retention) ([]string, error) {
 		return nil, fmt.Errorf("the restore points could not be listed: %w", err)
 	}
 	var ids, letGo []string
-	for _, s := range handLetGo(snaps, b.Now(), keep.Hand) {
+	for _, s := range handLetGo(snaps, b.GoodNights(), b.Now(), keep.Hand) {
 		ids, letGo = append(ids, s.ID), append(letGo, s.ShortID)
 	}
 	if len(ids) > 0 {
@@ -524,6 +532,7 @@ func (b *Backups) BackedUp(ctx context.Context, module string) ([]ModuleBackups,
 	measured := b.Measurements()
 	snaps, _ := b.Snapshots(ctx, module)
 	inForce := b.inForce(ctx)
+	goodNights := b.GoodNights()
 	thinned := b.LastRun().Thinned
 	out := []ModuleBackups{}
 	listed := map[string]bool{}
@@ -534,7 +543,7 @@ func (b *Backups) BackedUp(ctx context.Context, module string) ([]ModuleBackups,
 		listed[d.Module] = true
 		m := ModuleBackups{Module: d.Module, Runs: len(d.Runs), Paths: d.Paths, Retention: inForce, Thinned: thinned,
 			Data:      itemReports(items[d.Module], measured[d.Module], d.Paths, good[d.Module]),
-			HandTaken: handKept(d.Module, snaps, inForce)}
+			HandTaken: handKept(d.Module, snaps, goodNights, inForce)}
 		if n, ok := nights[d.Module]; ok {
 			m.LastNight = &n
 		}

modules/restic/cmd/restic-backups/hand.go

@@ -13,7 +13,10 @@ package main
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
+	"os"
+	"path/filepath"
 	"slices"
 	"strings"
 	"time"
@@ -40,25 +43,48 @@ func moduleOf(s Snapshot) string {
 	return ""
 }
 
-// isNight is whether a restore point is a night's: its module's tag and nothing else — neither taken by hand
-// nor a retired item's last one. Restic writes a snapshot only for a backup that succeeded, so each is good.
-func isNight(s Snapshot) bool { return len(s.Tags) == 1 && moduleOf(s) != "" }
+// goodNightsFile is each module's newest night this program recorded as good, by its restore point's short id.
+// A snapshot is no proof of a good night: restic writes one, and exits 3, when a source file could not be read,
+// and that night is recorded as NOT backed up (one). Nor is Good, which counts a backup taken by hand too.
+const goodNightsFile = "good-nights.json"
 
-// latestNights is the time of each module's newest night.
-func latestNights(snaps []Snapshot) map[string]time.Time {
+// GoodNights is each module's newest good night's restore point; empty before the first since novox/hq issue
+// 453, and until then every restore point taken by hand is kept.
+func (b *Backups) GoodNights() map[string]string {
+	out := map[string]string{}
+	if raw, err := os.ReadFile(filepath.Join(b.Where.State, goodNightsFile)); err == nil {
+		_ = json.Unmarshal(raw, &out)
+	}
+	return out
+}
+
+func (b *Backups) recordGoodNight(module, snapshot string) {
+	good := b.GoodNights()
+	good[module] = snapshot
+	raw, _ := json.MarshalIndent(good, "", "  ")
+	if err := os.WriteFile(filepath.Join(b.Where.State, goodNightsFile), append(raw, '\n'), 0o600); err != nil {
+		b.Say("recording %s's good night failed: %v", module, err)
+	}
+}
+
+// latestNights is the time of each module's newest good night, read from its restore point: a module whose
+// good night's restore point is not listed has none.
+func latestNights(snaps []Snapshot, goodNights map[string]string) map[string]time.Time {
 	latest := map[string]time.Time{}
 	for _, s := range snaps {
-		if m := moduleOf(s); isNight(s) && s.Time.After(latest[m]) {
+		m := moduleOf(s)
+		if id := goodNights[m]; m != "" && id != "" && (s.ShortID == id || strings.HasPrefix(s.ID, id)) && !isHand(s) {
 			latest[m] = s.Time
 		}
 	}
 	return latest
 }
 
-// handLetGo is the restore points taken by hand that the thinning forgets: older than days, and with a night of
-// the same module after them. Every other one taken by hand is kept.
-func handLetGo(snaps []Snapshot, now time.Time, days int) []Snapshot {
-	latest := latestNights(snaps)
+// handLetGo is the restore points taken by hand that the thinning forgets: older than days, and with a good night
+// of the same module after them. Every other one taken by hand is kept — one of a module that left this machine
+// for good, which no later night follows, too: deleting a module's last restore point is a person's act.
+func handLetGo(snaps []Snapshot, goodNights map[string]string, now time.Time, days int) []Snapshot {
+	latest := latestNights(snaps, goodNights)
 	cutoff := now.AddDate(0, 0, -days)
 	var out []Snapshot
 	for _, s := range snaps {
@@ -81,8 +107,8 @@ type HandKept struct {
 }
 
 // handKept is a module's restore points taken by hand, oldest first, said with the retention in force.
-func handKept(module string, snaps []Snapshot, keep InForce) []HandKept {
-	latest := latestNights(snaps)
+func handKept(module string, snaps []Snapshot, goodNights map[string]string, keep InForce) []HandKept {
+	latest := latestNights(snaps, goodNights)
 	out := []HandKept{}
 	for _, s := range snaps {
 		if !isHand(s) || moduleOf(s) != module {

modules/restic/cmd/restic-backups/hand_test.go

@@ -18,10 +18,13 @@ import (
 	"time"
 )
 
-// handRestic is a fake restic that remembers each command and lists the snapshots it is given.
+// handRestic is a fake restic that remembers each command and lists the snapshots it is given. A backup makes
+// the snapshot made (abcdef01 when empty), and fails as restic's exit 3 when partial.
 type handRestic struct {
-	calls []string
-	snaps []Snapshot
+	calls   []string
+	snaps   []Snapshot
+	made    string
+	partial bool
 }
 
 func (h *handRestic) run(_ context.Context, name string, args ...string) (string, error) {
@@ -32,7 +35,14 @@ func (h *handRestic) run(_ context.Context, name string, args ...string) (string
 	h.calls = append(h.calls, line)
 	switch args[5] {
 	case "backup":
-		return "{\"message_type\":\"summary\",\"snapshot_id\":\"abcdef0123456789\"}\n", nil
+		if h.partial {
+			return "", errors.New("Warning: at least one source file could not be read")
+		}
+		made := h.made
+		if made == "" {
+			made = "abcdef0123456789"
+		}
+		return "{\"message_type\":\"summary\",\"snapshot_id\":\"" + made + "\"}\n", nil
 	case "snapshots":
 		raw, _ := json.Marshal(h.snaps)
 		return string(raw), nil
@@ -97,8 +107,12 @@ func TestAHandTakenRestorePointIsLetGoOnlyPastItsBoundAndAfterALaterNight(t *tes
 		snap("otherhnd", at(11), "module=web", HandTag),          // a night of another module after it is no night of its own
 		snap("ret-hand", at(11), "module=web", "retired=x"),      // a retired restore point is no night either
 		snap("webnight", at(10.5), "module=web", "retired=data"), // nor this
+		snap("tvhand01", at(10), "module=tv", HandTag),           // past the bound; only a partial night of tv after it
+		snap("tvnight0", at(12), "module=tv"),                    // tv's good night, before it
+		snap("tvpartl1", at(1), "module=tv"),                     // a partial night: a snapshot, and NOT backed up
 	}
-	got := handLetGo(snaps, now, 7)
+	goodNights := map[string]string{"pg": "pgnight2", "mail": "mailnite", "tv": "tvnight0"}
+	got := handLetGo(snaps, goodNights, now, 7)
 	var ids []string
 	for _, s := range got {
 		ids = append(ids, s.ShortID)
@@ -112,7 +126,7 @@ func TestAHandTakenRestorePointIsLetGoOnlyPastItsBoundAndAfterALaterNight(t *tes
 // The thinning forgets a hand-taken restore point past its bound by its id, and records which.
 func TestTheThinningForgetsAnExpiredHandTakenRestorePointByItsId(t *testing.T) {
 	now := time.Date(2026, 10, 20, 3, 0, 0, 0, time.UTC)
-	h := &handRestic{snaps: []Snapshot{
+	h := &handRestic{made: "bbbbbbbb11", snaps: []Snapshot{
 		{ID: "aaaaaaaa11", ShortID: "aaaaaaaa", Time: now.AddDate(0, 0, -9), Tags: []string{"module=pg", HandTag}},
 		{ID: "bbbbbbbb11", ShortID: "bbbbbbbb", Time: now, Tags: []string{"module=pg"}},
 	}}
@@ -168,6 +182,7 @@ func TestBackedUpSaysWhichRestorePointsAreKeptAsHandTakenAndUntilWhen(t *testing
 		{ID: "cccccccc11", ShortID: "cccccccc", Time: late, Tags: []string{"module=pg", HandTag}},
 	}}
 	b := &Backups{Where: placed(t, "# pg\npath /\n"), Run: h.run, Now: func() time.Time { return now }, Say: quiet}
+	b.recordGoodNight("pg", "bbbbbbbb")
 	listed, err := b.BackedUp(context.Background(), "pg")
 	must(t, err)
 	if len(listed) != 1 || len(listed[0].HandTaken) != 2 {
@@ -254,3 +269,58 @@ func TestWithTheRealResticAHandTakenBackupOutlivesTheNightTheSameDay(t *testing.
 		t.Errorf("recorded %+v, want the hand-taken restore point let go", th)
 	}
 }
+
+// A partial night — restic exits 3, a source file could not be read, and still writes a snapshot — is recorded
+// as NOT backed up, and is no later night: a restore point taken by hand past its days is kept beside it.
+func TestAPartialNightIsNoLaterNightForARestorePointTakenByHand(t *testing.T) {
+	now := time.Date(2026, 10, 20, 3, 0, 0, 0, time.UTC)
+	h := &handRestic{partial: true, snaps: []Snapshot{
+		{ID: "nnnnnnnn11", ShortID: "nnnnnnnn", Time: now.AddDate(0, 0, -12), Tags: []string{"module=pg"}},
+		{ID: "aaaaaaaa11", ShortID: "aaaaaaaa", Time: now.AddDate(0, 0, -9), Tags: []string{"module=pg", HandTag}},
+		// What the partial night below left: a snapshot of pg alone.
+		{ID: "pppppppp11", ShortID: "pppppppp", Time: now, Tags: []string{"module=pg"}},
+	}}
+	b := &Backups{Where: placed(t, "# pg\npath /\n"), Run: h.run, Now: func() time.Time { return now }, Say: quiet}
+	b.recordGoodNight("pg", "nnnnnnnn")
+	outcome, err := b.BackUp(context.Background(), "")
+	must(t, err)
+	if outcome["pg"].OK {
+		t.Fatalf("the partial night is recorded good: %+v", outcome["pg"])
+	}
+	for _, f := range h.with("restic forget") {
+		if strings.Contains(f, "aaaaaaaa") {
+			t.Errorf("let go the restore point taken by hand on a partial night: %q", f)
+		}
+	}
+	listed, err := b.BackedUp(context.Background(), "pg")
+	must(t, err)
+	if len(listed[0].HandTaken) != 1 || !listed[0].HandTaken[0].WaitsForANight {
+		t.Errorf("hand-taken %+v, want it still waiting for a good night", listed[0].HandTaken)
+	}
+}
+
+// When the counts' thinning fails after restore points taken by hand were let go, those are said gone: never
+// "nothing was forgotten".
+func TestAThinningThatFailsAfterLettingGoSaysWhatItLetGo(t *testing.T) {
+	now := time.Date(2026, 10, 20, 3, 0, 0, 0, time.UTC)
+	h := &handRestic{made: "bbbbbbbb11", snaps: []Snapshot{
+		{ID: "aaaaaaaa11", ShortID: "aaaaaaaa", Time: now.AddDate(0, 0, -9), Tags: []string{"module=pg", HandTag}},
+		{ID: "bbbbbbbb11", ShortID: "bbbbbbbb", Time: now, Tags: []string{"module=pg"}},
+	}}
+	run := func(ctx context.Context, name string, args ...string) (string, error) {
+		if name == "restic" && args[5] == "forget" && len(args) > 6 && args[6] == "--prune" {
+			return "", errors.New("Fatal: repository is already locked")
+		}
+		return h.run(ctx, name, args...)
+	}
+	b := &Backups{Where: placed(t, "# pg\npath /\n"), Run: run, Now: func() time.Time { return now }, Say: quiet}
+	_, err := b.BackUp(context.Background(), "")
+	must(t, err)
+	if th := b.LastRun().Thinned; th == nil || th.OK || !slices.Equal(th.LetGo, []string{"aaaaaaaa"}) {
+		t.Fatalf("recorded %+v, want the failure and aaaaaaaa let go", th)
+	}
+	why := b.Health(context.Background()).Why
+	if strings.Contains(why, "Nothing was forgotten") || !strings.Contains(why, "let go aaaaaaaa") {
+		t.Errorf("said %q, want it to say aaaaaaaa was let go", why)
+	}
+}

modules/restic/cmd/restic-backups/health.go

@@ -106,9 +106,16 @@ func Judge(s Seen) Health {
 			`with node-backup.now {"module": "%s"} on this machine.`, m, clock(n.At), strings.TrimSuffix(n.Error, "."), since, m))
 	}
 	if t := s.Run.Thinned; t != nil && !t.OK && t.Error != "" {
-		found = append(found, fmt.Sprintf("Thinning the restore points failed at %s: %s. Nothing was forgotten and every "+
-			"restore point is kept, so the repository grows until a thinning succeeds; fix what the error names, then "+
-			"node-backup.now on this machine thins again.", clock(t.At), strings.TrimSuffix(t.Error, ".")))
+		// Restore points taken by hand are let go before the counts' thinning, so they are gone when it fails after
+		// (novox/hq issue 453): said, rather than "nothing was forgotten".
+		kept := "Nothing was forgotten and every restore point is kept"
+		if len(t.LetGo) > 0 {
+			kept = "It let go " + strings.Join(t.LetGo, ", ") + ", taken by hand, past their days and a later night; " +
+				"every other restore point is kept"
+		}
+		found = append(found, fmt.Sprintf("Thinning the restore points failed at %s: %s. %s, so the repository grows "+
+			"until a thinning succeeds; fix what the error names, then node-backup.now on this machine thins again.",
+			clock(t.At), strings.TrimSuffix(t.Error, "."), kept))
 	}
 	r := s.Retention
 	switch {

modules/restic/cmd/restic-backups/health_test.go

@@ -380,3 +380,24 @@ func TestANightWhoseLockCannotBeTakenIsRecorded(t *testing.T) {
 		t.Fatalf("%s: %q", h.State, h.Why)
 	}
 }
+
+// A file without hand= is one the mesh did not write as this manifest does (novox/hq issue 453): keep-hand-taken-days
+// has a default, so there is nothing for the operator to set; it says to push the machine.
+func TestARetentionWithoutHandSaysToPushTheMachine(t *testing.T) {
+	b, _ := nightAt(t)
+	must(t, os.WriteFile(filepath.Join(b.Where.State, RetentionFile), []byte("daily=14\nweekly=8\nmonthly=6\n"), 0o600))
+	h := b.Health(context.Background())
+	if h.State != health.StateUnhealthy || len(h.Waits) != 0 {
+		t.Fatalf("a file without hand= is %s waiting for %+v: %s", h.State, h.Waits, h.Why)
+	}
+	for _, want := range []string{"hand= line is missing", "mesh-controller.push"} {
+		if !strings.Contains(h.Why, want) {
+			t.Errorf("said %q, want it to hold %q", h.Why, want)
+		}
+	}
+	// An invalid value is still the operator's, naming the setting.
+	must(t, os.WriteFile(filepath.Join(b.Where.State, RetentionFile), []byte("daily=14\nweekly=8\nmonthly=6\nhand=0\n"), 0o600))
+	if h := b.Health(context.Background()); h.State != health.StateWaits || len(h.Waits) != 1 || h.Waits[0].Setting != "keep-hand-taken-days" {
+		t.Fatalf("hand=0 is %s waiting for %+v", h.State, h.Waits)
+	}
+}

modules/restic/cmd/restic-backups/retention.go

@@ -66,14 +66,23 @@ func ParseRetention(text string) (Retention, error) {
 		}
 		seen[k], _ = strconv.Atoi(v)
 	}
-	for _, k := range []string{"daily", "weekly", "monthly", "hand"} {
+	for _, k := range []string{"daily", "weekly", "monthly"} {
 		if seen[k] == 0 {
 			return Retention{}, &SettingRefused{Setting: settingOf[k]}
 		}
 	}
+	// keep-hand-taken-days has a default, so the mesh always writes hand=: a file without it was written by
+	// something other than this module's manifest — an older one, before novox/hq issue 453 — and pushing the
+	// machine writes it again. No setting for the operator to give.
+	if seen["hand"] == 0 {
+		return Retention{}, errors.New(HandMissing)
+	}
 	return Retention{seen["daily"], seen["weekly"], seen["monthly"], seen["hand"]}, nil
 }
 
+// HandMissing is said of a retention file without the hand= line the mesh writes from keep-hand-taken-days.
+const HandMissing = "the hand= line is missing, which the mesh writes from keep-hand-taken-days and its default"
+
 // SettingRefused is one retention setting the night refuses: the operator's to give again.
 type SettingRefused struct{ Setting, Value string }
 

modules/restic/cmd/restic-backups/retention_test.go

@@ -146,7 +146,7 @@ func TestARetentionThatIsNotACountThinsNothingAndSaysWhich(t *testing.T) {
 		{"daily=14\ndaily=3\nweekly=8\nmonthly=6\n", "daily"},
 		{"daily 14\nweekly=8\nmonthly=6\n", "key=value"},
 		{"daily=14\nweekly=8\nmonthly=6\nhand=0\n", "keep-hand-taken-days"},
-		{"daily=14\nweekly=8\nmonthly=6\n", "keep-hand-taken-days"},
+		{"daily=14\nweekly=8\nmonthly=6\n", "hand= line is missing"},
 	} {
 		got, said := thinning(t, &c.conf)
 		if got != "" {