Wait for the batch that took a folded walk over, and let a delivery superseded by its walk follow that walk (issue 1786)

modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go

@@ -1396,17 +1396,24 @@ func (h *Holder) plan() []func() {
 	// Following a walk that was taken over: the walk that took it over, read as soon as it is heard of, asked for
 	// by its id when it is not.
 	for _, d := range h.deliveries {
-		if d.State.Final() || d.Walk == nil || d.Walk.TakenOverBy == "" {
+		if d.Walk == nil || d.Walk.TakenOverBy == "" || d.State.Final() && !d.supersededByItsWalk() {
 			continue
 		}
 		if s, known := h.walks[d.Walk.TakenOverBy]; known {
+			if s.walk.IsBatch() {
+				// A batch is no walk yet (ADR 0276): the walk it is cut into answers the merge. Waited for, never
+				// read as a walk that does not answer it (issue 1786).
+				continue
+			}
 			if c, ok := walkAnswers(s.walk, d); ok {
 				h.linkWalk(d, s.walk, c)
 			} else {
 				// The walk that took its own over does not answer it: superseded, as before ADR 0276.
 				h.Logf("[mesh-delivery] %s: the walk %s, which took %s over, does not answer its merge", d.ID,
 					d.Walk.TakenOverBy, d.Walk.ID)
-				d.Walk.TakenOverBy = ""
+				if !d.State.Final() {
+					d.Walk.TakenOverBy = ""
+				}
 			}
 			h.settleAndKeep(d)
 			continue

modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1786_test.go

@@ -0,0 +1,99 @@
+package main
+
+import (
+	"testing"
+	"time"
+)
+
+// novox/hq issue 1786: two merges whose walks the controller folded, before they were cut, into a batch that was
+// still queued (not yet cut into a walk) ended superseded: the batch is no walk that answers a merge, so the follow
+// loop took the walk's taken_over_by off ("the walk that took its own over does not answer it") and the table's
+// supersession then held ("names no walk that took it over"). Delivered by the batch's walk, they showed
+// superseded for good. A batch that took a walk over is waited for; and one already ended so follows its taker.
+
+const (
+	head1786  = "1786178617861786178617861786178617861786"
+	merge1786 = "a1786a1786a1786a1786a1786a1786a1786a17860"
+)
+
+// folded1786 is a delivery whose walk was folded into a batch that is still queued.
+func folded1786(t *testing.T) (*world, string) {
+	t.Helper()
+	w := newWorld(t)
+	id := IDOf("novox/app", head1786)
+	w.h.PullUpdated(pr("novox/app", 7, head1786, "feat/x", "src/a.go"))
+	w.h.Checked(verdict("novox/app", 7, head1786, "pass"))
+	w.h.PullMerged(merged("novox/app", 7, head1786, merge1786))
+	w.h.WalkMoved(w.ctl.put(aWalk("plan-own", "novox/app", merge1786, true)))
+	w.settleAll()
+	if s := w.state(id); s != Delivering {
+		t.Fatalf("setup: %s, want delivering", s)
+	}
+	closes := w.now.Add(time.Minute)
+	batch := Walk{ID: "plan-batch", Repository: "novox/other", Branch: "main", Commit: "ffff", Created: w.now, State: walkQueued,
+		Delivery: &WalkDelivery{Awaits: "mesh-delivery", Batch: &WalkBatch{ClosesAt: &closes},
+			Merges: []WalkMerge{{Repository: "novox/app", Commit: merge1786}}}}
+	w.h.WalkMoved(w.ctl.put(batch))
+	folded := w.ctl.walk("plan-own")
+	folded.State = walkSuperseded
+	folded.Note = "folded into plan-batch before it was cut"
+	folded.Delivery.TakenOverBy = "plan-batch"
+	w.h.WalkMoved(w.ctl.put(folded))
+	w.settleAll()
+	return w, id
+}
+
+func cut1786(w *world, state string) {
+	b := w.ctl.walk("plan-batch")
+	b.State = state
+	b.Delivery.Batch = nil
+	b.Tiers, b.Modules = [][]string{{"app"}}, map[string]*WalkModule{"app": {}}
+	w.h.WalkMoved(w.ctl.put(b))
+	w.settleAll()
+}
+
+func TestAWalkFoldedIntoAQueuedBatchIsNotSupersededAndFollowsTheBatchWhenItIsCut(t *testing.T) {
+	w, id := folded1786(t)
+	if d := w.delivery(id); d.State.Final() || d.Walk.TakenOverBy != "plan-batch" {
+		t.Fatalf("folded into a queued batch it is %s, taken over by %q; wanted it still following plan-batch", d.State, d.Walk.TakenOverBy)
+	}
+	cut1786(w, walkBuilding)
+	cut1786(w, walkDone)
+	if s := w.state(id); s != Delivered {
+		t.Fatalf("the batch's walk is done and it is %s; wanted delivered", s)
+	}
+}
+
+func TestADeliverySupersededWhileItsTakerWasABatchFollowsTheTakerOnceItEnds(t *testing.T) {
+	w, id := folded1786(t)
+	d := w.delivery(id)
+	// The state kept by the fault: superseded, its walk record naming the walk that took it over.
+	d.State = Superseded
+	d.Transitions = append(d.Transitions, Transition{At: w.now, From: Delivering, To: Superseded, Event: EvSuperseded})
+	if err := w.h.Store.PutDelivery(d); err != nil {
+		t.Fatal(err)
+	}
+	cut1786(w, walkBuilding)
+	cut1786(w, walkDone)
+	w.later(time30s)
+	w.settleAll()
+	if s := w.state(id); s != Delivered {
+		t.Fatalf("superseded by the fault and its taker done, it is %s; wanted delivered", s)
+	}
+}
+
+func TestASupersededDeliveryWithNoWalkThatTookItOverStaysSuperseded(t *testing.T) {
+	w := newWorld(t)
+	id := IDOf("novox/app", head1786)
+	w.h.PullUpdated(pr("novox/app", 7, head1786, "feat/x", "src/a.go"))
+	w.h.PullUpdated(pr("novox/app", 7, "b"+head1786[1:], "feat/x", "src/a.go"))
+	w.settleAll()
+	if s := w.state(id); s != Superseded {
+		t.Fatalf("a head superseded by a newer one is %s", s)
+	}
+	w.later(time30s)
+	w.settleAll()
+	if s := w.state(id); s != Superseded {
+		t.Fatalf("it moved: %s", s)
+	}
+}

modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table.go

@@ -86,6 +86,8 @@ const (
 	EvLifted          Event = "lifted"           // what held it for a person is no longer so: its group's composed check passed, or a walk now answers its merge
 	EvRewalk          Event = "rewalk"           // the walk that carried a later commit for it failed, or was stopped for another merge (ADR 0299): it waits for the walk that walks it again
 	EvBackupFailed    Event = "backup failed"    // a backup before the send failed or stayed silent: nothing of its plan was sent (novox/hq ADR 0404)
+	EvFollowed        Event = "followed"         // its walk was superseded by a walk that took it over, which has ended: it follows that walk's end (issue 1786)
+	EvFollowedFailed  Event = "followed failed"  // the same, and that walk failed
 	EvTrunkMoved      Event = "trunk moved"      // another merge moved its trunk, and its verdict names the trunk commit before (novox/hq ADR 0323)
 )
 
@@ -395,6 +397,23 @@ var Table = []Row{
 		Says: func(d *Delivery) string {
 			return "covered: a later merge that contains it was delivered: " + d.Carrier
 		}},
+	{From: []State{Superseded}, Event: EvFollowed, To: Delivered,
+		Guard: "it was ended superseded by its walk, which a walk took over (issue 1786), and the walk it follows now is done, carrying its own commit",
+		Holds: func(d *Delivery, f Facts) error {
+			if !d.supersededByItsWalk() {
+				return errors.New("it was not ended by a walk superseded")
+			}
+			return unless(d.Walk != nil && d.Walk.State == walkDone && !d.carriedLater(), "the walk it follows is not done, or carried a later commit for it")
+		}},
+	{From: []State{Superseded}, Event: EvFollowedFailed, To: Failed,
+		Guard: "it was ended superseded by its walk, which a walk took over (issue 1786), and the walk it follows now failed, carrying its own commit",
+		Holds: func(d *Delivery, f Facts) error {
+			if !d.supersededByItsWalk() {
+				return errors.New("it was not ended by a walk superseded")
+			}
+			return unless(d.Walk != nil && d.Walk.State == walkFailed && d.Walk.StoppedBy == "" && !d.carriedLater(),
+				"the walk it follows did not fail, or carried a later commit for it")
+		}},
 	{From: []State{Delivering}, Event: EvStop, To: Stopped,
 		Guard: "its walk was stopped through this owner, and walks its merge again nowhere: left out, or the walk ended " +
 			"whole (novox/hq ADR 0299)",
@@ -733,3 +752,14 @@ func TableText() map[string]any {
 	}
 	return map[string]any{"transitions": rows, "bounds": bounds, "machine-steps": steps}
 }
+
+// supersededByItsWalk is whether a delivery ended superseded because its walk was: its last move was a supersession
+// from published or delivering, not a newer head of its pull request's (issue 1786). Such a delivery follows the walk
+// that took its walk over, once that is heard, whatever it was ended as before.
+func (d *Delivery) supersededByItsWalk() bool {
+	if d.State != Superseded || d.Walk == nil || len(d.Transitions) == 0 {
+		return false
+	}
+	t := d.Transitions[len(d.Transitions)-1]
+	return t.Event == EvSuperseded && (t.From == Published || t.From == Delivering)
+}

modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table_test.go

@@ -95,6 +95,13 @@ func aDeliveryFor(r Row) (*Delivery, Facts) {
 		d.Walk = &WalkSeen{ID: "plan-1", State: walkFailed}
 	case EvSuperseded:
 		d.Walk = &WalkSeen{ID: "plan-1", State: walkSuperseded}
+	case EvFollowed, EvFollowedFailed:
+		// Ended superseded by its walk, which a walk took over (issue 1786); that walk has ended now.
+		d.Transitions = []Transition{{From: Delivering, To: Superseded, Event: EvSuperseded}}
+		d.Walk = &WalkSeen{ID: "plan-2", State: walkDone}
+		if r.Event == EvFollowedFailed {
+			d.Walk.State = walkFailed
+		}
 	}
 	return d, f
 }
@@ -104,12 +111,13 @@ func TestEveryRowOfTheTableIsTakenWhenItsGuardHoldsAndRefusedWhenNot(t *testing.
 		for _, from := range r.From {
 			d, f := aDeliveryFor(r)
 			d.State = from
+			kept := len(d.Transitions)
 			got, err := Apply(d, r.Event, r.Act, f)
 			if err != nil {
 				t.Errorf("%s —%s→ %s was refused with its guard holding: %v", orNothing(from), r.Event, r.To, err)
 				continue
 			}
-			if d.State != r.To || got.From != from || got.To != r.To || len(d.Transitions) != 1 {
+			if d.State != r.To || got.From != from || got.To != r.To || len(d.Transitions) != kept+1 {
 				t.Errorf("%s —%s→ left it %s, transition %+v", orNothing(from), r.Event, d.State, got)
 			}
 			// The guard not holding: a delivery with no facts.
@@ -135,7 +143,7 @@ func TestEveryRowOfTheTableIsTakenWhenItsGuardHoldsAndRefusedWhenNot(t *testing.
 func TestEveryPairTheTableDoesNotHoldIsRefusedByName(t *testing.T) {
 	events := []Event{EvAnnounced, EvAppeared, EvAdopted, EvChecked, EvAccepted, EvRefused, EvRecheck, EvReask, EvNewHead,
 		EvClosed, EvMerged, EvMergedUnchecked, EvGo, EvHold, EvRelease, EvDone, EvFailed, EvSuperseded, EvStop,
-		EvHistory, EvRetire, EvCovered, EvLifted, EvRewalk, EvTrunkMoved, EvBackupFailed}
+		EvHistory, EvRetire, EvCovered, EvLifted, EvRewalk, EvTrunkMoved, EvBackupFailed, EvFollowed, EvFollowedFailed}
 	for _, from := range append([]State{None}, AllStates...) {
 		for _, ev := range events {
 			for _, act := range []bool{false, true} {
@@ -162,7 +170,8 @@ func TestTheTableKeepsItsRules(t *testing.T) {
 	for _, r := range Table {
 		for _, from := range r.From {
 			// Nothing leaves a final state.
-			if from.Final() {
+			// The one exception: a delivery superseded by its walk follows the walk that took it over (issue 1786).
+			if from.Final() && r.Event != EvFollowed && r.Event != EvFollowedFailed {
 				t.Errorf("%s is final and the table leaves it by %s", from, r.Event)
 			}
 			// Nothing goes back before the trunk once a delivery is on it.