Judge a packaging module's news by plans that built it, over every plan since its build (hq ADR 0267, review)

A plan that closed without building a module hid a missed merge from it;
a window of recent plans let an old closure back in once the plan that
overtook it slid out; a merge the forge gave no time was acted on again
every pass. A plan answering the merge's own commit is now a look at it,
and clocks a little apart do not make a merge history.

cmd/mesh-controller/missed_merges.go

@@ -102,6 +102,11 @@ func catchUpOnMerges(ctx context.Context, now time.Time, announced merges,
 		return err
 	}
 	for _, a := range all {
+		// A merge the forge said no time of is dated by its announcement, so a packaging module's look can
+		// make it history once acted on, and the catch-up does not act on it again every pass (ADR 0267).
+		if a.SourceMoved.MergedAt == "" && !a.At.IsZero() {
+			a.SourceMoved.MergedAt = a.At.UTC().Format(time.RFC3339)
+		}
 		if now.Sub(a.At) < mergeGrace {
 			continue
 		}

cmd/mesh-controller/planner_rules_test.go

@@ -403,11 +403,29 @@ func TestAMissedMergeMovingOnlyAPackagingModuleIsActedOnOnce(t *testing.T) {
 	if got := wouldMove(m, entries, planningView(read, nil)); len(got) != 1 || got[0].Manifest.Module != "route-proxy" {
 		t.Fatalf("a missed merge of the proxy's program would move %v", got)
 	}
-	acted := []inventory.Plan{{State: inventory.PlanBuilding, Created: merged.Add(time.Minute),
+	// Acted on at once: the plan answering this very merge is a look, however close the clocks.
+	atOnce := []inventory.Plan{{State: inventory.PlanBuilding, Commit: "c1", Created: merged.Add(2 * time.Second),
+		Modules: map[string]*inventory.PlanModule{"route-proxy": {State: "building"}}}}
+	if got := wouldMove(m, entries, planningView(read, atOnce)); len(got) != 0 {
+		t.Fatalf("a merge whose own plan holds the proxy would move %v again", got)
+	}
+	acted := []inventory.Plan{{State: inventory.PlanBuilding, Created: merged.Add(2 * time.Minute),
 		Modules: map[string]*inventory.PlanModule{"route-proxy": {State: "building"}}}}
 	if got := wouldMove(m, entries, planningView(read, acted)); len(got) != 0 {
 		t.Fatalf("a merge acted on for the proxy would move %v again", got)
 	}
+	// A plan that closed without building it looked at nothing: the merge is still news for it.
+	closed := []inventory.Plan{{State: "failed", Created: merged.Add(2 * time.Minute),
+		Modules: map[string]*inventory.PlanModule{"route-proxy": {State: "waiting"}}}}
+	if got := wouldMove(m, entries, planningView(read, closed)); len(got) != 1 {
+		t.Fatalf("a plan that never built the proxy hid the merge from it: %v", got)
+	}
+	// A look just before the merge, on clocks a little apart, is no look after it.
+	skewed := []inventory.Plan{{State: inventory.PlanBuilding, Created: merged.Add(30 * time.Second),
+		Modules: map[string]*inventory.PlanModule{"route-proxy": {State: "building"}}}}
+	if got := wouldMove(m, entries, planningView(read, skewed)); len(got) != 1 {
+		t.Fatalf("a look within the clocks' margin made the merge history: %v", got)
+	}
 }
 
 // sharedRepositoryEdges is what dependenciesOf derives for the catalogue of the test above, sorted as it

cmd/mesh-controller/upgrades.go

@@ -569,7 +569,13 @@ func mergeCandidates(m link.SourceMoved, entries []inventory.Entry,
 			// build or plan of it after the merge already read the repository with the merge in it. Per
 			// module, since a merge that moved only the module built from the repository says nothing
 			// about the ones packaging it.
-			if isHistory(m.MergedAt, lookedOf(read[e.Manifest.Module])) {
+			// Judged with a margin for the forge's clock running behind the store's: too late a look
+			// rebuilds once more, too early one would miss the merge.
+			if lookedAtCommit(read[e.Manifest.Module], m.Commit) {
+				continue
+			}
+			if looked := lookedOf(read[e.Manifest.Module]); !looked.IsZero() &&
+				isHistory(m.MergedAt, looked.Add(-historyMargin)) {
 				continue
 			}
 			packaging = append(packaging, e)
@@ -578,6 +584,19 @@ func mergeCandidates(m link.SourceMoved, entries []inventory.Entry,
 	return from, packaging, already
 }
 
+// lookedAtCommit is whether a plan that built a module, or is building it, answered this merge commit.
+func lookedAtCommit(read []inventory.ReadRepository, commit string) bool {
+	for _, r := range read {
+		if slices.Contains(r.LookedAt, commit) {
+			return true
+		}
+	}
+	return false
+}
+
+// historyMargin is how far a packaging module's last look is taken back before a merge is history for it.
+const historyMargin = time.Minute
+
 // lookedOf is when a module packaging another repository was last looked at, as readForPlanning says.
 func lookedOf(read []inventory.ReadRepository) time.Time {
 	var at time.Time
@@ -761,8 +780,9 @@ func staleIn(read map[string][]inventory.ReadRepository, plans []inventory.Plan)
 }
 
 // lookedAt is when a merge was last acted on for a module that packages another repository's source: its
-// newest build, or the newest plan that held it, whichever is later. A build asked after a merge clones that
-// repository with the merge in it, so an older merge is history for it.
+// newest build, or the newest plan that built it or is still building it, whichever is later. A build asked
+// after a merge clones that repository with the merge in it, so an older merge is history for it; a plan
+// that closed without building it looked at nothing.
 func lookedAt(name string, read []inventory.ReadRepository, plans []inventory.Plan) time.Time {
 	var at time.Time
 	for _, r := range read {
@@ -771,7 +791,8 @@ func lookedAt(name string, read []inventory.ReadRepository, plans []inventory.Pl
 		}
 	}
 	for _, p := range plans {
-		if _, in := p.Modules[name]; in && p.Created.After(at) {
+		s, in := p.Modules[name]
+		if in && (p.Open() || (s != nil && s.State == "built")) && p.Created.After(at) {
 			at = p.Created
 		}
 	}
@@ -787,22 +808,35 @@ func readForPlanning(ctx context.Context, inv *inventory.Inventory) (map[string]
 	if err != nil {
 		return nil, err
 	}
-	plans, err := inv.RecentPlans(ctx, planLookBack)
+	// Every plan since the oldest build whose source is recorded: one made after a module's build can have
+	// overtaken it, however long ago, so no window of recent plans would do.
+	var oldest time.Time
+	for _, rs := range read {
+		for _, r := range rs {
+			if !r.Built.IsZero() && (oldest.IsZero() || r.Built.Before(oldest)) {
+				oldest = r.Built
+			}
+		}
+	}
+	plans, err := inv.PlansSince(ctx, oldest)
 	if err != nil {
 		return nil, err
 	}
 	return planningView(read, plans), nil
 }
 
-// planLookBack is how many recent plans judge whether a recorded build source is overtaken.
-const planLookBack = 500
-
 // planningView is readForPlanning over what was read, so a test can hand it records.
 func planningView(read map[string][]inventory.ReadRepository, plans []inventory.Plan) map[string][]inventory.ReadRepository {
 	stale := staleIn(read, plans)
 	out := make(map[string][]inventory.ReadRepository, len(read))
 	for name, rs := range read {
 		looked := lookedAt(name, rs, plans)
+		var commits []string
+		for _, p := range plans {
+			if st, in := p.Modules[name]; in && p.Commit != "" && (p.Open() || (st != nil && st.State == "built")) {
+				commits = append(commits, p.Commit)
+			}
+		}
 		var kept []inventory.ReadRepository
 		for _, r := range rs {
 			if stale[name] {
@@ -811,7 +845,7 @@ func planningView(read map[string][]inventory.ReadRepository, plans []inventory.
 				}
 				r.Paths = nil
 			}
-			r.Looked = looked
+			r.Looked, r.LookedAt = looked, commits
 			kept = append(kept, r)
 		}
 		out[name] = kept