Keep a merge a cut made history, and build a batch's walk at the branch (hq ADR 0276 review)

cmd/mesh-controller/batches.go

@@ -153,7 +153,11 @@ func (f following) hearMerge(ctx context.Context, m link.SourceMoved, now time.T
 			return notNow(err)
 		}
 	}
-	if from, packaging, already := mergeCandidates(m, entries, read); len(from) == 0 && len(packaging) == 0 && len(later) == 0 {
+	owed, err := owedLate(ctx, inv, m, merged, touches)
+	if err != nil {
+		return notNow(err)
+	}
+	if from, packaging, already := mergeCandidates(m, entries, read); len(from) == 0 && len(packaging) == 0 && len(later) == 0 && !owed {
 		// "Already built from it" and "nothing reads it" are different facts, and reading the first
 		// as the second sends somebody looking for a broken trigger when the mesh is up to date.
 		if already > 0 {
@@ -216,6 +220,23 @@ func (f following) hearMerge(ctx context.Context, m link.SourceMoved, now time.T
 	return cutBatchesHeld(ctx, f.open, now)
 }
 
+// owedLate says a merge that reads as history is still the record's to answer (review of this change): a cut
+// marks every module it moved seen at that moment, so a merge of that branch the bus lost and the catch-up
+// hands over later — made before the cut, heard after it — reads as history and was dropped. One that touches
+// a module the mesh holds, of a branch whose merges the controller keeps, made since it began keeping them,
+// is kept; one made before it began is old news, as before.
+func owedLate(ctx context.Context, inv *inventory.Inventory, m link.SourceMoved, merged time.Time,
+	touches []inventory.Entry) (bool, error) {
+	if len(touches) == 0 {
+		return false, nil
+	}
+	kept, first, err := inv.KeptOnBranch(ctx, m.Owner+"/"+m.Repo, m.Base)
+	if err != nil || !kept {
+		return false, err
+	}
+	return !merged.Before(first.Add(-mergeGrace)), nil
+}
+
 // answeredByALaterMerge is the plan that answers a merge heard after a later merge of its branch was put in
 // one (ADR 0276 decision 4): a batch not yet cut, whose walk plans every file of the repository's merges; an
 // open walk, or a walk done, that builds every module this merge would move — built from the branch after the
@@ -595,7 +616,7 @@ func walkAlone(ctx context.Context, open *stores, m inventory.BatchedMerge, now
 	p := inventory.Plan{ID: fmt.Sprintf("plan-%d", now.UnixNano()), Repository: m.Repository, Branch: m.Branch,
 		Commit: m.Commit, Merged: m.Merged, Created: now, State: inventory.PlanQueued, Tiers: [][]string{},
 		Modules: map[string]*inventory.PlanModule{}, Note: "walked alone after a failed walk carried it",
-		Delivery: &inventory.PlanDelivery{}}
+		Delivery: &inventory.PlanDelivery{Alone: true}}
 	if err := inv.SavePlan(ctx, &p); err != nil {
 		return err
 	}
@@ -678,6 +699,7 @@ func planBatch(ctx context.Context, open *stores, batch *inventory.Plan, carry [
 		plan.Delivery = &inventory.PlanDelivery{}
 	}
 	plan.Delivery.Merges = named
+	plan.Delivery.Alone = batch.Delivery != nil && batch.Delivery.Alone
 	if len(moved) == 0 {
 		plan.State = inventory.PlanDone
 		plan.Tiers = [][]string{}

cmd/mesh-controller/batches_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"io"
 	"os"
@@ -138,8 +139,8 @@ func TestTwoMergesSecondsApartAreOneWalkAtTheLaterCommit(t *testing.T) {
 	if w.Delivery == nil || !slices.Equal(w.Delivery.Merges, want) {
 		t.Fatalf("the walk answers %+v, want %+v", w.Delivery, want)
 	}
-	if len(*asked) != 2 || (*asked)[0][2] != dunst.Commit || (*asked)[1][2] != dunst.Commit {
-		t.Fatalf("the walk did not ask its modules at the commit it carries: %v", *asked)
+	if len(*asked) != 2 || (*asked)[0][2] != "main" || (*asked)[1][2] != "main" {
+		t.Fatalf("the walk did not ask its modules at the branch, which holds the commit it carries: %v", *asked)
 	}
 	// Heard again, as the bus may hand it over twice: never a second merge, nor a second walk.
 	hear(t, open, claude, t0.Add(5*time.Minute))
@@ -625,3 +626,46 @@ func TestARemovedFileIsTheLastMergesWord(t *testing.T) {
 		t.Fatalf("combined as %+v", got)
 	}
 }
+
+// The catch-up hands over what the bus lost after its branch's later merges were cut, and the record keeps it:
+// an earlier merge is named by the walk that carried it; a merge made before a cut and heard after it, which
+// no later merge carries, joins the next batch — neither reads as history and is dropped (review of this change).
+func TestTheCatchUpKeepsWhatACutMadeHistory(t *testing.T) {
+	open := windowed(t)
+	asksWithPaths(t)
+	ctx := t.Context()
+	base := time.Now().UTC().Add(-time.Hour).Truncate(time.Second)
+	hear(t, open, repoMerge("one", "c2", base.Add(10*time.Second)), base.Add(11*time.Second))
+	cutAt(t, open, base.Add(2*time.Minute))
+	ws, _ := walks(t, open)
+	w := ws[0]
+	w.State = inventory.PlanDone
+	if err := open.inventory.SavePlan(ctx, &w); err != nil {
+		t.Fatal(err)
+	}
+	lost := []link.AnnouncedMerge{
+		{SourceMoved: repoMerge("one", "c1", base), At: base.Add(time.Second)},
+		{SourceMoved: repoMerge("one", "c3lost", base.Add(20*time.Second)), At: base.Add(21 * time.Second)},
+	}
+	catalogued := func(ctx context.Context) ([]inventory.Entry, map[string][]inventory.ReadRepository, error) {
+		entries, err := open.inventory.Catalogued(ctx)
+		if err != nil {
+			return nil, nil, err
+		}
+		read, err := readForPlanning(ctx, open.inventory)
+		return entries, read, err
+	}
+	if err := catchUpOnMerges(ctx, time.Now(), unheardMerges{announcedList(lost), open.inventory}, catalogued,
+		(following{open}).SourceMoved, t.Logf); err != nil {
+		t.Fatal(err)
+	}
+	got, _ := open.inventory.PlanByID(ctx, w.ID)
+	if len(got.Delivery.Merges) != 2 || got.Delivery.Merges[0] != (inventory.PlanMerge{Repository: "novox/one",
+		Commit: "c1", Carried: "c2"}) {
+		t.Fatalf("the earlier merge the catch-up handed over is not named by the walk that carried it: %+v",
+			got.Delivery.Merges)
+	}
+	if _, kept, err := open.inventory.MergeOf(ctx, "novox/one", "c3lost"); err != nil || !kept {
+		t.Fatalf("the merge made before the cut and heard after it was dropped: %v %v", kept, err)
+	}
+}

cmd/mesh-controller/missed_merges.go

@@ -66,6 +66,24 @@ func (u unheardMerges) AnnouncedMerges(ctx context.Context, since time.Time) ([]
 	return out, nil
 }
 
+// owedToTheRecord says a merge read as history is still owed to the record (novox/hq ADR 0276): the merges
+// reader knows, when it keeps them (unheardMerges).
+func owedToTheRecord(ctx context.Context, announced merges, m link.SourceMoved, entries []inventory.Entry,
+	read map[string][]inventory.ReadRepository) bool {
+	u, ok := announced.(unheardMerges)
+	if !ok {
+		return false
+	}
+	merged, err := time.Parse(time.RFC3339Nano, m.MergedAt)
+	if err != nil {
+		return false
+	}
+	raw := m
+	raw.MergedAt = ""
+	owed, err := owedLate(ctx, u.inv, m, merged, wouldMove(raw, entries, read))
+	return err == nil && owed
+}
+
 // catchingUpOnMerges reads back the forge's announcements on a timer, until the context ends.
 func catchingUpOnMerges(ctx context.Context, open *stores, announced merges) {
 	f := following{open}
@@ -136,7 +154,7 @@ func catchUpOnMerges(ctx context.Context, now time.Time, announced merges,
 		if now.Sub(a.At) < mergeGrace {
 			continue
 		}
-		if len(wouldMove(a.SourceMoved, entries, read)) == 0 {
+		if len(wouldMove(a.SourceMoved, entries, read)) == 0 && !owedToTheRecord(ctx, announced, a.SourceMoved, entries, read) {
 			continue
 		}
 		if entries, read, err = catalogued(ctx); err != nil {
@@ -144,7 +162,12 @@ func catchUpOnMerges(ctx context.Context, now time.Time, announced merges,
 		}
 		moves := wouldMove(a.SourceMoved, entries, read)
 		if len(moves) == 0 {
-			continue
+			if !owedToTheRecord(ctx, announced, a.SourceMoved, entries, read) {
+				continue
+			}
+			raw := a.SourceMoved
+			raw.MergedAt = ""
+			moves = wouldMove(raw, entries, read)
 		}
 		var names []string
 		for _, e := range moves {

cmd/mesh-controller/release_plan.go

@@ -330,12 +330,12 @@ func askModule(ctx context.Context, p *inventory.Plan, name string, byName map[s
 	}
 	source := buildSource{Repository: e.Source.Repository, Seat: e.Source.Seat}
 	fmt.Printf("  tier %d: ", p.Tier)
-	// The branch it follows, never a commit a build once named (novox/hq 04-ISSUES/215) — but for the commit a
-	// batch's walk carries of the module's own repository and branch (novox/hq ADR 0276): the walk builds what
-	// it answers, not a merge heard after it was cut, and a merge walked alone is built on its own commit.
+	// The branch it follows, never a commit a build once named (novox/hq 04-ISSUES/215): the branch contains every
+	// commit a batch's walk carries — but for a merge walked alone after a failed walk (novox/hq ADR 0276
+	// decision 3), built on its own commit to find which merge brought the failure.
 	ref := followedBranch(e.Source.Ref)
 	carried := p.CommitOn(e.Source.Repository, ref)
-	if len(p.Commits) > 0 && carried != "" {
+	if p.Delivery != nil && p.Delivery.Alone && carried != "" {
 		ref = carried
 	}
 	id, err := askABuild(ctx, source, e.Source.Path, ref)