Walk a stopped walk's merges again in the open batch, in one act, and say what stays on the machines (review of #232, hq issue 459)

A new batch beside the open one walked an older commit after a newer one, and a
stop that failed partway could leave a merge on two plans. A module shared with
the stopped merge, built at its branch, would deliver it; it is left out or
withheld. A batch not yet cut can drop a merge, so mesh-delivery can stop a
merge walked again. The stopped walk names what it left on which machines.

cmd/mesh-controller/issue459_test.go

@@ -1,7 +1,10 @@
 package main
 
 import (
+	"encoding/json"
+	"errors"
 	"reflect"
+	"slices"
 	"strings"
 	"testing"
 	"time"
@@ -52,6 +55,49 @@ func TestLeftOutAreTheStoppedMergeAndTheMergesThatContainIt(t *testing.T) {
 	}
 }
 
+// moving is a kept merge that moved modules when it was heard.
+func moving(m inventory.BatchedMerge, modules ...string) inventory.BatchedMerge {
+	m.Event, _ = json.Marshal(keptMerge{Moves: modules})
+	return m
+}
+
+// A merge of another repository that moves a module the stopped merge moves is left out too, naming the modules
+// it shares: the module is built at its branch, which holds the stopped merge (review of #232). And so on, until
+// none more is: a merge sharing a module with that one is left out as well.
+func TestAMergeSharingAStoppedMergesModuleIsLeftOut(t *testing.T) {
+	stopped := moving(leaved("novox/mesh-catalog", "c1aaaaaaaaaa", t0), "app")
+	reader := moving(leaved("novox/one", "o1bbbbbbbbbb", t0.Add(time.Second)), "app", "one")
+	next := moving(leaved("novox/two", "t1cccccccccc", t0.Add(2*time.Second)), "one")
+	free := moving(leaved("novox/three", "h1dddddddddd", t0.Add(3*time.Second)), "three")
+	again, out, err := leaveOut([]inventory.BatchedMerge{stopped, reader, next, free}, []string{"novox/mesh-catalog@c1aaaaaaa"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(again) != 1 || again[0].Commit != free.Commit {
+		t.Fatalf("walked again: %+v; want the merge that shares nothing alone", again)
+	}
+	want := []inventory.PlanLeftOut{{Repository: "novox/mesh-catalog", Commit: stopped.Commit},
+		{Repository: "novox/one", Commit: reader.Commit, Contains: stopped.Commit, Shares: []string{"app"}},
+		{Repository: "novox/two", Commit: next.Commit, Contains: stopped.Commit, Shares: []string{"one"}}}
+	if !reflect.DeepEqual(out, want) {
+		t.Fatalf("left out %+v; want %+v", out, want)
+	}
+}
+
+// A walk withholds a module a stopped merge moved where it would build it only as a dependent; one a merge of its
+// own batch moves is built, and empty tiers go.
+func TestAWalkWithholdsAStoppedMergesDependents(t *testing.T) {
+	plan := inventory.Plan{Tiers: [][]string{{"one"}, {"app"}, {"notes", "app2"}},
+		Modules: map[string]*inventory.PlanModule{"one": {}, "app": {}, "notes": {}, "app2": {}}}
+	got := withhold(&plan, []string{"app", "notes", "absent"}, []string{"notes"})
+	if !reflect.DeepEqual(got, []string{"app"}) || !reflect.DeepEqual(plan.Tiers, [][]string{{"one"}, {"notes", "app2"}}) {
+		t.Fatalf("withheld %v, tiers %v", got, plan.Tiers)
+	}
+	if _, in := plan.Modules["app"]; in {
+		t.Fatal("a withheld module is still in the plan")
+	}
+}
+
 // The seat verb passes the merges to leave out to the command.
 func TestDeliveryStopTakesTheMergesToLeaveOut(t *testing.T) {
 	got, err := argvFor("delivery-stop", map[string]any{"plan": "plan-1", "why": "w", "by": "jochen",
@@ -175,3 +221,173 @@ func TestAStopWithoutAMergeEndsTheMergesThatContainIt(t *testing.T) {
 		t.Fatalf("a batch of nothing was made: %+v", bs)
 	}
 }
+
+// A merge heard while the walk ran waits in the open batch, at a later commit of the same repository. The merge
+// walked again joins that batch, never a batch of its own that would walk the older commit after the newer one
+// (review of #232): its window closes, and its walk is at the newer commit, answering both.
+func TestAStopWalksTheOthersAgainInTheOpenBatch(t *testing.T) {
+	open := windowed(t)
+	asksRecorded(t)
+	ctx := t.Context()
+	cat := catalogueMerge("cat1aaaaaaaa", "app", t0)
+	one1 := repoMerge("one", "one1bbbbbbbb", t0.Add(5*time.Second))
+	hear(t, open, cat, t0.Add(time.Second))
+	hear(t, open, one1, t0.Add(6*time.Second))
+	cutAt(t, open, t0.Add(2*time.Minute))
+	ws, _ := walks(t, open)
+	walk := ws[0]
+	one2 := repoMerge("one", "one2cccccccc", t0.Add(3*time.Minute))
+	hear(t, open, one2, t0.Add(3*time.Minute))
+	_, bs := walks(t, open)
+	if len(bs) != 1 {
+		t.Fatalf("the merge heard while the walk ran is in %d batches", len(bs))
+	}
+	waiting := bs[0]
+	p, said, err := stopWalkWithout(ctx, open.inventory, walk.ID, "mesh-delivery for jochen", "its group stopped",
+		[]string{"novox/mesh-catalog@" + cat.Commit}, t0.Add(3*time.Minute+10*time.Second))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if p.Delivery.WalkedAgainBy != waiting.ID || said.WalkedAgainBy != waiting.ID {
+		t.Fatalf("walked again by %q (said %q); want the open batch %s", p.Delivery.WalkedAgainBy, said.WalkedAgainBy, waiting.ID)
+	}
+	if _, bs := walks(t, open); len(bs) != 1 {
+		t.Fatalf("%d batches; want the open one alone", len(bs))
+	}
+	b, _ := open.inventory.PlanByID(ctx, waiting.ID)
+	if b.Delivery.Batch == nil || !b.Delivery.Batch.Closed || !slices.Contains(b.Delivery.Batch.Withheld, "app") {
+		t.Fatalf("the open batch reads %+v; want its window closed and app withheld", b.Delivery.Batch)
+	}
+	// Its window closed by the stop, not by the clock: cut at once.
+	cutAt(t, open, t0.Add(3*time.Minute+11*time.Second))
+	fresh, _ := open.inventory.PlanByID(ctx, waiting.ID)
+	if fresh.Batch() || fresh.CommitOf("novox/one") != one2.Commit {
+		t.Fatalf("the batch is %s at %q; want it cut, walking the newer commit", fresh.State, fresh.CommitOf("novox/one"))
+	}
+	var answered []string
+	for _, m := range fresh.Delivery.Merges {
+		answered = append(answered, m.Commit+">"+m.Carried)
+	}
+	if !slices.Contains(answered, one1.Commit+">"+one2.Commit) || !slices.Contains(answered, one2.Commit+">") {
+		t.Fatalf("the walk answers %v; want the earlier merge carried by the later", answered)
+	}
+}
+
+// A stop names what the stopped walk already sent and nothing walks again, on which machines; and a later merge of
+// the stopped merge's repository that waits in a batch, which will deliver the stopped change. A stopped walk
+// that walked its merges again is refused a retry, saying why.
+func TestAStopNamesWhatStaysOnTheMachinesAndWhatStillCarriesIt(t *testing.T) {
+	open := windowed(t)
+	asksRecorded(t)
+	ctx := t.Context()
+	cat := catalogueMerge("cat1aaaaaaaa", "app", t0)
+	one1 := repoMerge("one", "one1bbbbbbbb", t0.Add(5*time.Second))
+	hear(t, open, cat, t0.Add(time.Second))
+	hear(t, open, one1, t0.Add(6*time.Second))
+	cutAt(t, open, t0.Add(2*time.Minute))
+	ws, _ := walks(t, open)
+	walk := ws[0]
+	sent := t0.Add(150 * time.Second)
+	for name, m := range walk.Modules {
+		if name == "app" || name == "one" {
+			m.First, m.FirstAt = []string{"anchor"}, &sent
+		}
+	}
+	if err := open.inventory.SavePlan(ctx, &walk); err != nil {
+		t.Fatal(err)
+	}
+	cat2 := catalogueMerge("cat2dddddddd", "notes", t0.Add(3*time.Minute))
+	hear(t, open, cat2, t0.Add(3*time.Minute))
+	_, said, err := stopWalkWithout(ctx, open.inventory, walk.ID, "mesh-delivery for jochen", "its group stopped",
+		[]string{"novox/mesh-catalog@" + cat.Commit}, t0.Add(3*time.Minute+10*time.Second))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !reflect.DeepEqual(said.Kept, []inventory.PlanKept{{Module: "app", Machines: []string{"anchor"}}}) {
+		t.Fatalf("kept %+v; want app on anchor alone (one is walked again)", said.Kept)
+	}
+	p, _ := open.inventory.PlanByID(ctx, walk.ID)
+	if !reflect.DeepEqual(p.Delivery.Kept, said.Kept) || !strings.Contains(p.Note, "app on anchor") {
+		t.Fatalf("the stopped walk's record does not name what stays: %+v %q", p.Delivery.Kept, p.Note)
+	}
+	if len(said.StillCarriedBy) != 1 || !strings.Contains(said.StillCarriedBy[0], cat2.Commit) {
+		t.Fatalf("still carried by %v; want the later catalogue merge waiting in the batch", said.StillCarriedBy)
+	}
+	if _, err := retryPlan(ctx, open, walk.ID); err == nil || !strings.Contains(err.Error(), "was stopped without") {
+		t.Fatalf("a walk stopped without some merges was retried, or refused for another reason: %v", err)
+	}
+}
+
+// A batch not yet cut is stopped without a merge: the merge goes to a stopped record of its own, the batch goes on
+// with the others and withholds the stopped merge's modules; with every merge left out, the batch is stopped.
+func TestAStopWithoutAMergeLeavesItOutOfABatch(t *testing.T) {
+	open := windowed(t)
+	asksRecorded(t)
+	ctx := t.Context()
+	cat := catalogueMerge("cat1aaaaaaaa", "app", t0)
+	one := repoMerge("one", "one1bbbbbbbb", t0.Add(5*time.Second))
+	hear(t, open, cat, t0.Add(time.Second))
+	hear(t, open, one, t0.Add(6*time.Second))
+	_, bs := walks(t, open)
+	b := bs[0]
+	r, said, err := stopWalkWithout(ctx, open.inventory, b.ID, "mesh-delivery for jochen", "its group stopped",
+		[]string{"novox/mesh-catalog@" + cat.Commit}, t0.Add(10*time.Second))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if said.WalkedAgainBy != b.ID || said.Stopped == b.ID {
+		t.Fatalf("said %+v; want a stopped record of its own, and the batch walking the others", said)
+	}
+	if left, _ := open.inventory.MergesOf(ctx, said.Stopped); len(left) != 1 || left[0].Commit != cat.Commit {
+		t.Fatalf("the stopped record holds %+v", left)
+	}
+	if stopped, _ := open.inventory.PlanByID(ctx, said.Stopped); stopped.State != inventory.PlanFailed ||
+		stopped.Delivery.Stopped == "" || stopped.Delivery.WalkedAgainBy != b.ID {
+		t.Fatalf("the stopped record reads %s %+v", stopped.State, stopped.Delivery)
+	}
+	_ = r
+	cutAt(t, open, t0.Add(3*time.Minute))
+	w, _ := open.inventory.PlanByID(ctx, b.ID)
+	if _, in := w.Modules["app"]; in || w.Batch() {
+		t.Fatalf("the batch's walk is %s and builds %v; want it cut without app", w.State, w.Modules)
+	}
+	// A batch whose every merge is left out is stopped itself.
+	hear(t, open, catalogueMerge("cat3eeeeeeee", "notes", t0.Add(4*time.Minute)), t0.Add(4*time.Minute))
+	_, bs = walks(t, open)
+	last := bs[0]
+	if _, said, err := stopWalkWithout(ctx, open.inventory, last.ID, "mesh-delivery for jochen", "x",
+		[]string{"novox/mesh-catalog@cat3eeeeeeee"}, t0.Add(4*time.Minute+time.Second)); err != nil || said.Stopped != last.ID {
+		t.Fatalf("a batch left with nothing: %+v %v", said, err)
+	}
+	if p, _ := open.inventory.PlanByID(ctx, last.ID); p.State != inventory.PlanFailed || p.Batch() {
+		t.Fatalf("the emptied batch is %s", p.State)
+	}
+}
+
+// The stopped walk, the batch and the merges moved are one act (ADR 0299 decision 7): a plan written by somebody
+// else since it was read refuses all of it, and no merge moves, no plan is written.
+func TestSavingPlansAndMovingMergesIsOneAct(t *testing.T) {
+	open := windowed(t)
+	ctx := t.Context()
+	hear(t, open, catalogueMerge("cat1aaaaaaaa", "app", t0), t0.Add(time.Second))
+	_, bs := walks(t, open)
+	stale := bs[0]
+	moved := stale
+	moved.Note = "written by somebody else"
+	if err := open.inventory.SavePlan(ctx, &moved); err != nil {
+		t.Fatal(err)
+	}
+	fresh := inventory.Plan{ID: "plan-fresh", Repository: "novox/mesh-catalog", Branch: "main", Commit: "x", Created: t0,
+		State: inventory.PlanAssembling, Tiers: [][]string{}, Modules: map[string]*inventory.PlanModule{}}
+	err := open.inventory.SavePlansMoving(ctx, []*inventory.Plan{&fresh, &stale},
+		[]inventory.MergeMove{{Repository: "novox/mesh-catalog", Commit: "cat1aaaaaaaa", Plan: "plan-fresh"}})
+	if !errors.Is(err, inventory.ErrPlanMoved) {
+		t.Fatalf("a stale plan in the act: %v", err)
+	}
+	if _, err := open.inventory.PlanByID(ctx, "plan-fresh"); err == nil {
+		t.Fatal("the new plan was written though the act was refused")
+	}
+	if m, _, _ := open.inventory.MergeOf(ctx, "novox/mesh-catalog", "cat1aaaaaaaa"); m.Plan != stale.ID {
+		t.Fatalf("the merge moved to %q though the act was refused", m.Plan)
+	}
+}