morning, routine: reject the two configs that silently do nothing (V-581)

Both Due functions key their last-fired map by routine name, so two routines sharing a name took turns suppressing each other and one of them never fired. Validate now rejects a duplicate name in either package.

parseHHMM checked the digits arithmetically, which let a stray character cancel out: window_start of 2 :00 loaded as 04:00 and passed the validation that exists to catch that typo. Each of the four positions is now checked as a digit, which makes the negative bounds unreachable and they are gone.

Folded the three copies of the unevidenced-item loop in Evaluate, Outstanding and Due into one helper.

internal/morning/morning.go

@@ -104,14 +104,22 @@ func OptionalOnly(missing []Item) []Item {
 }
 
 // Validate reports the first structural problem with a routine set: missing
-// name/items, an unparseable HH:MM, an inverted window, a duplicate item key
-// within a routine, or an out-of-range weekday. Called at config load so a
-// typo surfaces at startup, not as a silently-broken checklist at runtime.
+// name/items, an unparseable HH:MM, an inverted window, a duplicate routine or
+// item key, or an out-of-range weekday. Called at config load so a typo
+// surfaces at startup, not as a silently-broken checklist at runtime.
+//
+// Routine names must be unique because Due keys its once-a-day map by name. Two
+// routines sharing one would take turns suppressing each other's nudge.
 func Validate(routines []Routine) error {
+	names := make(map[string]bool, len(routines))
 	for _, r := range routines {
 		if r.Name == "" {
 			return fmt.Errorf("morning: name is required")
 		}
+		if names[r.Name] {
+			return fmt.Errorf("morning routine %q: duplicate name", r.Name)
+		}
+		names[r.Name] = true
 		if len(r.Items) == 0 {
 			return fmt.Errorf("morning routine %q: at least one item is required", r.Name)
 		}
@@ -177,6 +185,17 @@ func Evaluate(r Routine, facts map[string]store.Fact, now time.Time) Status {
 	return st
 }
 
+// missing lists the items of r with no evidence in [start, now].
+func missing(r Routine, facts map[string]store.Fact, start, now time.Time) []Item {
+	var out []Item
+	for _, it := range r.Items {
+		if !evidenced(it, facts, start, now) {
+			out = append(out, it)
+		}
+	}
+	return out
+}
+
 // Outstanding reports the items of a routine that today has no evidence for,
 // whether or not the window is still open. Evaluate answers "what is missing
 // right now" and goes silent the moment the window closes; the day plan asks a
@@ -191,13 +210,7 @@ func Outstanding(r Routine, facts map[string]store.Fact, now time.Time) []Item {
 	if !ok || now.Before(start) {
 		return nil
 	}
-	var missing []Item
-	for _, it := range r.Items {
-		if !evidenced(it, facts, start, now) {
-			missing = append(missing, it)
-		}
-	}
-	return missing
+	return missing(r, facts, start, now)
 }
 
 // Due returns the routines that have reached their nudge time today with at
@@ -222,24 +235,19 @@ func Due(routines []Routine, facts map[string]store.Fact, last map[string]time.T
 		if !ok || now.Before(nudgeAt) {
 			continue
 		}
-		var missing []Item
-		for _, it := range r.Items {
-			if !evidenced(it, facts, start, now) {
-				missing = append(missing, it)
-			}
-		}
+		skipped := missing(r, facts, start, now)
 		// A day where only the optional items were skipped is a fine day, and
 		// nagging about it is what teaches him to stop listening (Vikunja
 		// #473). The optional ones still travel in Missing so the message can
 		// mention them when it is being sent anyway.
-		if len(Required(missing)) == 0 {
+		if len(Required(skipped)) == 0 {
 			continue
 		}
 		if prev, seen := last[r.Name]; seen && sameDay(prev, now) {
 			continue
 		}
 		last[r.Name] = now
-		out = append(out, Candidate{Routine: r, Missing: missing})
+		out = append(out, Candidate{Routine: r, Missing: skipped})
 	}
 	return out
 }
@@ -284,13 +292,21 @@ func sameDay(a, b time.Time) bool {
 	return ay == by && am == bm && ad == bd
 }
 
+// parseHHMM reads a five-character "HH:MM". Every digit is checked as a digit:
+// arithmetic alone lets a stray character cancel out, so "2 :00" used to load
+// as 04:00 and Validate passed the typo it exists to catch.
 func parseHHMM(s string) (hour, min int, ok bool) {
 	if len(s) != 5 || s[2] != ':' {
 		return 0, 0, false
 	}
+	for _, i := range [4]int{0, 1, 3, 4} {
+		if s[i] < '0' || s[i] > '9' {
+			return 0, 0, false
+		}
+	}
 	h := int(s[0]-'0')*10 + int(s[1]-'0')
 	m := int(s[3]-'0')*10 + int(s[4]-'0')
-	if h < 0 || h > 23 || m < 0 || m > 59 {
+	if h > 23 || m > 59 {
 		return 0, 0, false
 	}
 	return h, m, true

internal/morning/morning_test.go

@@ -56,6 +56,19 @@ func TestValidate(t *testing.T) {
 	if err := Validate([]Routine{bad}); err == nil {
 		t.Fatal("expected error for duplicate item key")
 	}
+
+	// Due keys its once-a-day map by name, so two routines sharing one would
+	// suppress each other's nudge instead of both firing.
+	if err := Validate([]Routine{r, r}); err == nil {
+		t.Fatal("expected error for duplicate routine name")
+	}
+
+	// Arithmetic alone let a stray character cancel out: "2 :00" read as 04:00.
+	bad = r
+	bad.WindowStart = "2 :00"
+	if err := Validate([]Routine{bad}); err == nil {
+		t.Fatal("expected error for a non-digit in window_start")
+	}
 }
 
 func TestEvaluateInactiveOutsideWindow(t *testing.T) {

internal/morning/plan.go

@@ -113,12 +113,12 @@ func BuildPlan(routines []Routine, facts map[string]store.Fact, events, reminder
 func checklistEntries(routines []Routine, facts map[string]store.Fact, now time.Time) []PlanEntry {
 	var out []PlanEntry
 	for _, r := range routines {
-		missing := Outstanding(r, facts, now)
-		if len(missing) == 0 {
+		left := Outstanding(r, facts, now)
+		if len(left) == 0 {
 			continue
 		}
-		labels := make([]string, 0, len(missing))
-		for _, it := range missing {
+		labels := make([]string, 0, len(left))
+		for _, it := range left {
 			label := it.Label
 			if label == "" {
 				label = it.Key

internal/routine/routine.go

@@ -38,13 +38,22 @@ type Routine struct {
 }
 
 // Validate reports the first structural problem with a routine set: a missing
-// name/cron/body or an unparseable cron expression. Called at config load so a
-// typo surfaces at startup, not as a silently-never-firing routine at runtime.
+// name/cron/body, a duplicate name or an unparseable cron expression. Called at
+// config load so a typo surfaces at startup, not as a silently-never-firing
+// routine at runtime.
+//
+// Names must be unique because Due keys its last-fired map by name. Two
+// routines sharing one would take turns being suppressed by each other's fire.
 func Validate(routines []Routine) error {
+	seen := make(map[string]bool, len(routines))
 	for _, r := range routines {
 		if r.Name == "" {
 			return fmt.Errorf("routine: name is required")
 		}
+		if seen[r.Name] {
+			return fmt.Errorf("routine %q: duplicate name", r.Name)
+		}
+		seen[r.Name] = true
 		if r.Body == "" {
 			return fmt.Errorf("routine %q: body is required", r.Name)
 		}

internal/routine/routine_test.go

@@ -60,6 +60,12 @@ func TestValidate(t *testing.T) {
 			}
 		})
 	}
+
+	// Due keys its last-fired map by name, so two routines sharing one would
+	// take turns being suppressed by the other's fire.
+	if err := Validate(append(ok, ok[0])); err == nil {
+		t.Error("expected an error for a duplicate name, got nil")
+	}
 }
 
 func TestDue(t *testing.T) {