mesh-delivery: never ship a stopped merge walked again, and stop only once the walk is (review of #221, hq issue 459)

A merge walked again into a batch and then stopped by its group stayed in that
batch, which was later let go. Any walk or batch that names a stopped
delivery's merge is now asked to leave it out and is never let go; a refused
leave-out stops the whole walk. A person's stop marks the delivery stopped only
after its walk is, and the answer says what stays on the machines and what
will still carry the change.

modules/mesh-delivery/cmd/mesh-delivery/answers.go

@@ -382,6 +382,13 @@ func (h *Holder) words() []func() {
 			continue
 		}
 		ids, why := append([]string(nil), want.ids...), strings.Join(want.whys, "; ")
+		// **Never the word for a walk that carries a stopped change** (ADR 0299, review of #221): it is asked to
+		// leave the merge out first (leaveStopped).
+		if stopped, _ := h.carriesStopped(walk); len(stopped) > 0 {
+			h.Logf("[mesh-delivery] the walk %s is not let go: it carries %s, whose delivery was stopped", walk,
+				strings.Join(stopped, ", "))
+			continue
+		}
 		if h.asking[walk] {
 			continue // its word is being asked
 		}

modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go

@@ -96,8 +96,12 @@ type fakeController struct {
 	stops    []string
 	// withouts are the stops that left merges out (novox/hq ADR 0299): "<walk> without <merge>,…".
 	withouts []string
-	plans    int
-	down     bool
+	// refuseWithout refuses every stop with merges to leave out, as a controller before ADR 0299 does; mergedAt are
+	// the merges' times, by commit, for what a later merge contains.
+	refuseWithout bool
+	mergedAt      map[string]time.Time
+	plans         int
+	down          bool
 	// onCheck runs as a check is asked, before it is answered: what the forge held at that moment.
 	onCheck func()
 	// planned is what Plan answers instead of "builds app", when set.
@@ -105,7 +109,7 @@ type fakeController struct {
 }
 
 func newFakeController() *fakeController {
-	return &fakeController{walks: map[string]Walk{}, held: true}
+	return &fakeController{walks: map[string]Walk{}, held: true, mergedAt: map[string]time.Time{}}
 }
 
 func (c *fakeController) Plan(repository, base, head string, paths, dirs, removed []string) (*DeliveryPlan, error) {
@@ -179,7 +183,10 @@ func (c *fakeController) Stop(walk, why, by string) error {
 		return errors.New("down")
 	}
 	c.stops = append(c.stops, walk)
-	if w, known := c.walks[walk]; known {
+	if w, known := c.walks[walk]; known && !w.Open() {
+		// As the controller: only a walk that is building or rolling is stopped (`delivery-stop`).
+		return fmt.Errorf("%s is already %s", walk, w.State)
+	} else if known {
 		w.State = walkFailed
 		if w.Delivery == nil {
 			w.Delivery = &WalkDelivery{}
@@ -192,8 +199,10 @@ func (c *fakeController) Stop(walk, why, by string) error {
 }
 
 // StopWithout ends a walk without the merges named, as the controller does (novox/hq ADR 0299): each merge named,
-// and each merge of its repository whose commit the walk carried for it, is left out; the others are walked again
-// by a walk of their own, "<walk>-again", waiting for its word.
+// and each later merge of its repository by merge time (mergedAt), is left out; the others are put in a batch,
+// "<walk>-again", assembling until a test cuts it. Given a batch, the merges left out go to a stopped record,
+// "<walk>-left", and the batch goes on with the others. refuseWithout refuses every such stop, as an older
+// controller does.
 func (c *fakeController) StopWithout(walk, why, by string, without []string) (StopAnswer, error) {
 	c.mu.Lock()
 	defer c.mu.Unlock()
@@ -201,46 +210,64 @@ func (c *fakeController) StopWithout(walk, why, by string, without []string) (St
 		return StopAnswer{}, errors.New("down")
 	}
 	c.withouts = append(c.withouts, walk+" without "+strings.Join(without, ","))
+	if c.refuseWithout {
+		return StopAnswer{}, errors.New(`delivery-stop takes no argument "without"`)
+	}
 	w, known := c.walks[walk]
-	if !known || w.State == walkFailed || w.State == walkDone {
-		return StopAnswer{}, fmt.Errorf("%s is no open walk", walk)
+	if !known || !(w.Open() || w.IsBatch()) {
+		return StopAnswer{}, fmt.Errorf("%s is no open walk or batch", walk)
 	}
 	wd := WalkDelivery{}
 	if w.Delivery != nil {
 		wd = *w.Delivery
 	}
 	named := func(m WalkMerge) bool { return containsString(without, m.Repository+"@"+m.Commit) }
-	a := StopAnswer{Stopped: walk}
-	var again []WalkMerge
-	for _, m := range wd.Merges {
-		if named(m) {
-			a.LeftOut = append(a.LeftOut, WalkLeftOut{Repository: m.Repository, Commit: m.Commit})
-		}
-	}
-	for _, m := range wd.Merges {
-		if named(m) {
-			continue
-		}
-		contains := ""
+	contains := func(m WalkMerge) string {
 		for _, s := range wd.Merges {
-			if named(s) && s.Repository == m.Repository && s.Carried == m.Commit {
-				contains = s.Commit
+			if named(s) && s.Repository == m.Repository && c.mergedAt[m.Commit].After(c.mergedAt[s.Commit]) {
+				return s.Commit
 			}
 		}
-		if contains != "" {
-			a.LeftOut = append(a.LeftOut, WalkLeftOut{Repository: m.Repository, Commit: m.Commit, Contains: contains})
-			continue
+		return ""
+	}
+	a := StopAnswer{Stopped: walk}
+	var again, left []WalkMerge
+	for _, m := range wd.Merges {
+		switch {
+		case named(m):
+			a.LeftOut = append(a.LeftOut, WalkLeftOut{Repository: m.Repository, Commit: m.Commit})
+			left = append(left, m)
+		case contains(m) != "":
+			a.LeftOut = append(a.LeftOut, WalkLeftOut{Repository: m.Repository, Commit: m.Commit, Contains: contains(m)})
+			left = append(left, m)
+		default:
+			m.Carried = ""
+			again = append(again, m)
+			a.WalkedAgain = append(a.WalkedAgain, m.Repository+"@"+m.Commit)
 		}
-		m.Carried = ""
-		again = append(again, m)
-		a.WalkedAgain = append(a.WalkedAgain, m.Repository+"@"+m.Commit)
+	}
+	if len(a.LeftOut) == 0 {
+		return StopAnswer{}, fmt.Errorf("%s answers no merge %s", walk, strings.Join(without, ", "))
+	}
+	if w.IsBatch() {
+		rid := walk + "-left"
+		c.walks[rid] = Walk{ID: rid, Repository: left[0].Repository, Branch: "main", Commit: left[0].Commit,
+			Created: w.Created, State: walkFailed, Revision: 1, Modules: map[string]*WalkModule{},
+			Delivery: &WalkDelivery{Stopped: by, StoppedWhy: why, Merges: left, LeftOut: a.LeftOut, WalkedAgainBy: walk}}
+		wd.Merges = again
+		w.Delivery = &wd
+		w.Repository, w.Commit = again[0].Repository, again[0].Commit
+		w.Revision++
+		c.walks[walk] = w
+		a.Stopped, a.WalkedAgainBy = rid, walk
+		return a, nil
 	}
 	if len(again) > 0 {
 		id := walk + "-again"
+		closes := w.Created.Add(time.Minute)
 		c.walks[id] = Walk{ID: id, Repository: again[0].Repository, Branch: "main", Commit: again[0].Commit,
-			Created: w.Created.Add(time.Minute), State: walkBuilding, Tiers: [][]string{{"app"}},
-			Modules: map[string]*WalkModule{"app": {}}, Revision: 1,
-			Delivery: &WalkDelivery{Awaits: "mesh-delivery", Merges: again}}
+			Created: w.Created.Add(time.Minute), State: walkAssembling, Revision: 1,
+			Modules: map[string]*WalkModule{}, Delivery: &WalkDelivery{Merges: again, Batch: &WalkBatch{ClosesAt: &closes}}}
 		wd.WalkedAgainBy, a.WalkedAgainBy = id, id
 	}
 	wd.Stopped, wd.StoppedWhy, wd.LeftOut = by, why, a.LeftOut
@@ -251,6 +278,19 @@ func (c *fakeController) StopWithout(walk, why, by string, without []string) (St
 	c.walks[walk] = w
 	return a, nil
 }
+
+// cut makes a batch the walk under the same id, waiting for its word, as the controller cuts it.
+func (c *fakeController) cut(id string) Walk {
+	c.mu.Lock()
+	w := c.walks[id]
+	c.mu.Unlock()
+	wd := *w.Delivery
+	wd.Batch, wd.Awaits = nil, "mesh-delivery"
+	w.Delivery = &wd
+	w.State, w.Tiers, w.Modules = walkBuilding, [][]string{{"app"}}, map[string]*WalkModule{"app": {}}
+	return c.put(w)
+}
+
 func (c *fakeController) Walks(walk string) (bool, []Walk, error) {
 	c.mu.Lock()
 	defer c.mu.Unlock()

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

@@ -59,6 +59,8 @@ type Holder struct {
 	asking map[string]bool
 	// leaving are, by walk, the merges a stop gathered to leave out of it, asked once (novox/hq ADR 0299).
 	leaving map[string][]string
+	// leaveAsked is when a walk was last asked to leave out the stopped merges it carries (leaveStopped).
+	leaveAsked map[string]leaveAsk
 }
 
 // unmatched is a walk no delivery is known for yet: its merge not heard, or merged before this owner held.
@@ -832,6 +834,9 @@ func (h *Holder) Tick() {
 	for _, ask := range h.plan() {
 		ask()
 	}
+	for _, ask := range h.leaveStopped() {
+		ask()
+	}
 }
 
 // plan works out, under the lock, what to ask; each ask applies its own answer.
@@ -1835,10 +1840,19 @@ func (h *Holder) stopInGroup(d *Delivery, why string) func() {
 		return nil
 	}
 	h.moved(d, []Transition{t})
-	if d.Walk == nil || d.Walk.State == walkDone || d.Walk.State == walkFailed || d.Walk.State == walkSuperseded {
+	// Its open walk; or, its walk stopped for another merge, the batch or walk that walks its merge again (review
+	// of #221): a merge walked again and then stopped by its group is left out there too.
+	var walk string
+	switch {
+	case d.Walk == nil:
+		return nil
+	case d.Walk.State == walkBuilding || d.Walk.State == walkRolling:
+		walk = d.Walk.ID
+	case d.walkedAgain():
+		walk = d.Walk.WalkedAgainBy
+	default:
 		return nil
 	}
-	walk := d.Walk.ID
 	if !h.leave(walk, d) {
 		return nil // the first member left out of this walk asks for every one
 	}
@@ -1850,15 +1864,114 @@ func (h *Holder) stopInGroup(d *Delivery, why string) func() {
 			}
 			return
 		}
-		a, err := h.Controller.StopWithout(walk, why, byOwner, without)
-		if err != nil {
-			h.Logf("[mesh-delivery] the walk %s could not be stopped without %s: %v", walk, strings.Join(without, ", "), err)
-			return
-		}
-		h.Logf("[mesh-delivery] the walk %s is stopped without %s; %s", walk, strings.Join(without, ", "), againWords(a))
+		h.Logf("[mesh-delivery] %s", h.stopLeaving(walk, why, byOwner, without))
 	}
 }
 
+// stopLeaving asks the controller to stop a walk or batch without merges (ADR 0299) and says what came of it. A
+// refusal — an older controller takes no `without`, or the leave-out could not be made — falls back to stopping the
+// whole walk: the stopped change is held back, and every delivery on it ends stopped with the reason (review of
+// #221). Outside the lock.
+func (h *Holder) stopLeaving(walk, why, by string, without []string) string {
+	h.mu.Lock()
+	h.askedToLeave(walk)
+	h.mu.Unlock()
+	a, err := h.Controller.StopWithout(walk, why, by, without)
+	if err == nil {
+		return fmt.Sprintf("the walk %s is stopped without %s; %s", walk, strings.Join(without, ", "), againWords(a))
+	}
+	refused := err
+	if err := h.Controller.Stop(walk, fmt.Sprintf("%s, and it could not be left out alone (%v)", why, refused), by); err != nil {
+		return fmt.Sprintf("the walk %s could not be stopped without %s (%v), nor whole (%v): `plans stop %s --why …` "+
+			"ends it by hand", walk, strings.Join(without, ", "), refused, err, walk)
+	}
+	return fmt.Sprintf("the walk %s could not be stopped without %s (%v), so it was stopped whole: every delivery on it "+
+		"ends stopped", walk, strings.Join(without, ", "), refused)
+}
+
+// carriesStopped are the merges a walk or batch carries whose deliveries were stopped, as `<repository>@<commit>`,
+// and why the first was: a record that names a stopped delivery's merge and has not left it out would deliver a
+// stopped change (review of #221). Under the lock.
+func (h *Holder) carriesStopped(walk string) ([]string, string) {
+	s, known := h.walks[walk]
+	if !known || !(s.walk.Open() || s.walk.IsBatch()) || s.walk.Delivery == nil || len(s.walk.Delivery.Merges) == 0 {
+		return nil, ""
+	}
+	var out []string
+	why := ""
+	ids := make([]string, 0, len(h.deliveries))
+	for id := range h.deliveries {
+		ids = append(ids, id)
+	}
+	slices.Sort(ids)
+	for _, id := range ids {
+		d := h.deliveries[id]
+		if d.State != Stopped || d.MergedAs == "" {
+			continue
+		}
+		// By the merges it names alone: its own commit may be another's merge it no longer answers.
+		if !slices.ContainsFunc(s.walk.Delivery.Merges, func(m WalkMerge) bool {
+			return strings.EqualFold(m.Repository, d.Repository) && sameCommit(m.Commit, d.MergedAs)
+		}) {
+			continue
+		}
+		if _, out := s.walk.leftOut(d.Repository, d.MergedAs); out {
+			continue
+		}
+		out = append(out, d.Repository+"@"+d.MergedAs)
+		if why == "" {
+			why = fmt.Sprintf("%s was stopped: %s", d.ID, lastWhyOf(d))
+		}
+	}
+	return out, why
+}
+
+// lastWhyOf is the reason of a delivery's last transition.
+func lastWhyOf(d *Delivery) string {
+	if len(d.Transitions) == 0 {
+		return ""
+	}
+	return d.Transitions[len(d.Transitions)-1].Why
+}
+
+// leaveAsk is when a walk was last asked to leave merges out, and at which revision of its record.
+type leaveAsk struct {
+	at       time.Time
+	revision int64
+}
+
+// askedToLeave notes an ask to leave merges out of a walk, at its record's revision: under the lock.
+func (h *Holder) askedToLeave(walk string) {
+	if h.leaveAsked == nil {
+		h.leaveAsked = map[string]leaveAsk{}
+	}
+	h.leaveAsked[walk] = leaveAsk{at: h.Now(), revision: h.walks[walk].walk.Revision}
+}
+
+// leaveStopped asks, for every open walk or batch that carries a stopped delivery's merge, that the merge be left
+// out (ADR 0299, review of #221): a merge walked again and then stopped, or stopped while its batch assembled, never
+// ships. Asked once per record of it while it is carried; such a walk is never given its word (words).
+func (h *Holder) leaveStopped() []func() {
+	h.mu.Lock()
+	defer h.mu.Unlock()
+	now := h.Now()
+	var asks []func()
+	for id, seen := range h.walks {
+		without, why := h.carriesStopped(id)
+		if len(without) == 0 {
+			continue
+		}
+		// Once per record of it: the record that answers an ask is newer. Asked again after five minutes, should
+		// the ask have gone nowhere.
+		if a, asked := h.leaveAsked[id]; asked && seen.walk.Revision <= a.revision && now.Sub(a.at) < 5*time.Minute {
+			continue
+		}
+		walk := id
+		asks = append(asks, func() { h.Logf("[mesh-delivery] %s", h.stopLeaving(walk, why, byOwner, without)) })
+	}
+	return asks
+}
+
 // leave gathers a delivery's merge to leave out of a walk (ADR 0299), for a caller holding the lock; true for the
 // first of that walk, whose ask asks for all.
 func (h *Holder) leave(walk string, d *Delivery) bool {
@@ -1913,7 +2026,9 @@ func (h *Holder) othersOn(walk string, without []string) []string {
 	return keys
 }
 
-// againWords is what a stop that left merges out did, in a person's words.
+// againWords is what a stop that left merges out did, in a person's words: the walk that walks the others again,
+// what it left out (the controller's word on what contains a stopped merge), what stays on the machines, and what
+// will still deliver a stopped change (ADR 0299).
 func againWords(a StopAnswer) string {
 	var left []string
 	for _, o := range a.LeftOut {
@@ -1923,14 +2038,31 @@ func againWords(a StopAnswer) string {
 		}
 		left = append(left, s)
 	}
+	var out string
 	switch {
 	case a.WalkedAgainBy != "":
-		return fmt.Sprintf("%s walks its other merges again: %s; left out: %s", a.WalkedAgainBy,
+		out = fmt.Sprintf("%s walks its other merges again: %s; left out: %s", a.WalkedAgainBy,
 			strings.Join(a.WalkedAgain, ", "), orNone(strings.Join(left, ", ")))
 	case len(left) > 0:
-		return "left out: " + strings.Join(left, ", ") + "; nothing else to walk"
+		out = "left out: " + strings.Join(left, ", ") + "; nothing else to walk"
+	default:
+		out = "nothing else to walk"
 	}
-	return "the controller ended it whole: it named no walk that walks its other merges again"
+	if len(a.Withheld) > 0 {
+		out += "; its walk withholds " + strings.Join(a.Withheld, ", ")
+	}
+	if len(a.Kept) > 0 {
+		var kept []string
+		for _, k := range a.Kept {
+			kept = append(kept, k.Module+" on "+strings.Join(k.Machines, ", "))
+		}
+		out += "; what the stopped walk already sent stays on the machines: " + strings.Join(kept, "; ")
+	}
+	if len(a.StillCarriedBy) > 0 {
+		out += "; still to deliver the stopped change when walked, a revert's to hold back: " +
+			strings.Join(a.StillCarriedBy, ", ")
+	}
+	return out
 }
 
 // GroupState is a group's state, derived from its members and its composed check — never set apart (novox/hq

modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go

@@ -77,6 +77,12 @@ func (s *shape459) heard(walks ...string) {
 	s.settleAll()
 }
 
+// cutAgain is the controller cutting the batch that walks a stopped walk's merges again, as plan-moved says it.
+func (s *shape459) cutAgain(id string) {
+	s.h.WalkMoved(s.ctl.cut(id))
+	s.settleAll()
+}
+
 func (s *shape459) done(walk string) {
 	r := s.ctl.walk(walk)
 	sent := s.now
@@ -113,6 +119,10 @@ func TestAGroupStopWalksTheOtherDeliveriesAgainWithoutIt(t *testing.T) {
 		t.Fatalf("the controller was asked %v; want %q", s.ctl.withouts, want)
 	}
 	s.heard("plan-2", "plan-2-again")
+	if containsString(s.ctl.delivers, "plan-2-again") {
+		t.Fatal("a batch not yet cut was given its word")
+	}
+	s.cutAgain("plan-2-again")
 	z := s.delivery(s.ids["novox/z"])
 	if z.State.Final() || z.Walk == nil || z.Walk.ID != "plan-2-again" || !containsString(s.ctl.delivers, "plan-2-again") {
 		t.Fatalf("z is %s on %+v, let go %v; want it following plan-2-again, let go", z.State, z.Walk, s.ctl.delivers)
@@ -152,6 +162,7 @@ func TestAGroupStopEndsAMergeThatContainsTheStoppedOne(t *testing.T) {
 	s.retarget("novox/b2", "novox/b")
 	s.failFirst()
 	s.heard("plan-2", "plan-2-again")
+	s.cutAgain("plan-2-again")
 	b2 := s.delivery(s.ids["novox/b2"])
 	if b2.State != Stopped || !strings.Contains(lastWhy(b2), shortOf(s.merges["novox/b"], 8)) ||
 		!strings.Contains(lastWhy(b2), "contains") {
@@ -170,6 +181,9 @@ func (s *shape459) retarget(later, of string) {
 	d := s.h.deliveries[s.ids[later]]
 	d.Repository, d.MergedAt = of, d.MergedAt.Add(time.Minute)
 	s.h.mu.Unlock()
+	s.ctl.mu.Lock()
+	s.ctl.mergedAt[s.merges[of]], s.ctl.mergedAt[s.merges[later]] = d.MergedAt.Add(-time.Minute), d.MergedAt
+	s.ctl.mu.Unlock()
 	two := s.ctl.walk("plan-2")
 	merges := []WalkMerge{}
 	for _, m := range two.Delivery.Merges {
@@ -240,6 +254,7 @@ func TestAPersonsStopAgainWalksTheOthersAgain(t *testing.T) {
 		t.Fatalf("z is %s once plan-2 was stopped and walked again; want published, waiting for that walk", z.State)
 	}
 	s.heard("plan-2-again")
+	s.cutAgain("plan-2-again")
 	if z := s.delivery(s.ids["novox/z"]); z.Walk == nil || z.Walk.ID != "plan-2-again" || z.State != Delivering {
 		t.Fatalf("z is %s on %+v; want it delivering on plan-2-again", z.State, z.Walk)
 	}
@@ -263,3 +278,106 @@ func TestAPersonsStopOfTheWalkStopsEveryDelivery(t *testing.T) {
 		t.Fatalf("z is %s (%q); want it stopped, naming y's stop", z.State, lastWhy(z))
 	}
 }
+
+// The reviewer's case (review of #221): group [a, b]; y and z ride plan-2 with b. y is stopped `way: again`, and the
+// batch that walks plan-2's others again carries b and z. Then a fails: b is stopped by its group, and its merge is
+// left out of that batch — never shipped by it — and z alone is walked and delivered.
+func TestAMergeWalkedAgainAndThenStoppedByItsGroupNeverShips(t *testing.T) {
+	s := open459(t, []string{"novox/a", "novox/b"}, []string{"novox/y", "novox/z"})
+	if _, err := s.h.StopWay(s.ids["novox/y"], "not wanted after all", "jochen", "again"); err != nil {
+		t.Fatal(err)
+	}
+	s.heard("plan-2", "plan-2-again")
+	s.failFirst()
+	if b := s.state(s.ids["novox/b"]); b != Stopped {
+		t.Fatalf("b is %s", b)
+	}
+	if want := "plan-2-again without novox/b@" + s.merges["novox/b"]; !containsString(s.ctl.withouts, want) {
+		t.Fatalf("the controller was asked %v; want %q", s.ctl.withouts, want)
+	}
+	s.heard("plan-2-again", "plan-2-again-left")
+	s.cutAgain("plan-2-again")
+	again := s.ctl.walk("plan-2-again")
+	for _, m := range again.Delivery.Merges {
+		if m.Repository == "novox/b" {
+			t.Fatalf("the walk %s still carries b's stopped merge: %+v", again.ID, again.Delivery.Merges)
+		}
+	}
+	if !containsString(s.ctl.delivers, "plan-2-again") {
+		t.Fatalf("z's walk was not let go: %v", s.ctl.delivers)
+	}
+	s.done("plan-2-again")
+	if z := s.state(s.ids["novox/z"]); z != Delivered {
+		t.Fatalf("z is %s", z)
+	}
+}
+
+// Whatever brought it there, a walk that carries a stopped delivery's merge is asked to leave it out, and is never
+// given its word while it carries it.
+func TestAWalkCarryingAStoppedMergeIsAskedToLeaveItOutAndNeverLetGo(t *testing.T) {
+	s := shared459(t)
+	if _, err := s.h.StopWay(s.ids["novox/y"], "not wanted after all", "jochen", "again"); err != nil {
+		t.Fatal(err)
+	}
+	// A walk the controller made that carries y's merge anyway, waiting for its word, with z on it.
+	x := aWalk("plan-x", "novox/z", s.merges["novox/z"], true)
+	x.Created = x.Created.Add(10 * time.Minute)
+	x.Delivery.Merges = []WalkMerge{{Repository: "novox/y", Commit: s.merges["novox/y"]},
+		{Repository: "novox/z", Commit: s.merges["novox/z"]}}
+	s.ctl.refuseWithout = true // it cannot be left out: the walk is stopped whole
+	s.h.WalkMoved(s.ctl.put(x))
+	s.settleAll()
+	if want := "plan-x without novox/y@" + s.merges["novox/y"]; !containsString(s.ctl.withouts, want) {
+		t.Fatalf("the controller was asked %v; want %q", s.ctl.withouts, want)
+	}
+	if containsString(s.ctl.delivers, "plan-x") {
+		t.Fatalf("a walk carrying a stopped merge was let go: %v", s.ctl.delivers)
+	}
+	if !containsString(s.ctl.stops, "plan-x") {
+		t.Fatalf("refused the leave-out, the walk was not stopped whole: %v", s.ctl.stops)
+	}
+}
+
+// A group stop the controller refuses to make without the merge (an older controller) stops the whole walk: the
+// stopped change is held back, and the delivery riding with it ends stopped, saying why.
+func TestARefusedLeaveOutStopsTheWholeWalk(t *testing.T) {
+	s := open459(t, []string{"novox/a", "novox/b"}, []string{"novox/z"})
+	s.ctl.refuseWithout = true
+	s.failFirst()
+	if len(s.ctl.withouts) != 1 || !containsString(s.ctl.stops, "plan-2") {
+		t.Fatalf("asked %v, ended whole %v; want the leave-out tried, then plan-2 ended whole", s.ctl.withouts, s.ctl.stops)
+	}
+	s.heard("plan-2")
+	if z := s.delivery(s.ids["novox/z"]); z.State != Stopped || !strings.Contains(lastWhy(z), "could not be left out") {
+		t.Fatalf("z is %s (%q); want it stopped, saying the leave-out was refused", z.State, lastWhy(z))
+	}
+}
+
+// A person's stop the controller cannot make leaves the delivery as it was, and says how to go on.
+func TestAPersonsStopThatCannotBeMadeStopsNothing(t *testing.T) {
+	s := shared459(t)
+	s.ctl.refuseWithout = true
+	_, err := s.h.StopWay(s.ids["novox/y"], "not wanted after all", "jochen", "again")
+	if err == nil || !strings.Contains(err.Error(), "is not stopped") || !strings.Contains(err.Error(), "plans stop") {
+		t.Fatalf("the refused stop said %v", err)
+	}
+	if st := s.state(s.ids["novox/y"]); st != Delivering {
+		t.Fatalf("y is %s; want it delivering, its stop not made", st)
+	}
+}
+
+// What a stop that left merges out says: the walk that walks the others, what it left out and what each contains,
+// as the controller says it, what stays on which machines, and what will still deliver the stopped change.
+func TestAStopSaysWhatStaysAndWhatStillCarriesIt(t *testing.T) {
+	said := againWords(StopAnswer{Stopped: "plan-2", WalkedAgainBy: "plan-3", WalkedAgain: []string{"novox/z@zz"},
+		LeftOut: []WalkLeftOut{{Repository: "novox/b", Commit: "b1b1b1b1b1b1"},
+			{Repository: "novox/b", Commit: "b2b2b2b2b2b2", Contains: "b1b1b1b1b1b1"}},
+		Kept:           []WalkKept{{Module: "lab", Machines: []string{"anchor"}}},
+		StillCarriedBy: []string{"novox/b@b3b3 (in plan-4)"}})
+	for _, want := range []string{"plan-3", "novox/z@zz", "b2b2b2b2b2b2 (contains b1b1b1b1b1b1)", "lab on anchor",
+		"stays on the machines", "novox/b@b3b3 (in plan-4)", "revert"} {
+		if !strings.Contains(said, want) {
+			t.Fatalf("the stop said %q; want %q in it", said, want)
+		}
+	}
+}

modules/mesh-delivery/cmd/mesh-delivery/ports.go

@@ -280,6 +280,18 @@ type StopAnswer struct {
 	WalkedAgainBy string        `json:"walked_again_by,omitempty"`
 	WalkedAgain   []string      `json:"walked_again,omitempty"`
 	LeftOut       []WalkLeftOut `json:"left_out,omitempty"`
+	// Kept are the modules the stopped walk sent that nothing walks again, on the machines named; Withheld the
+	// modules the walk that walks the others withholds; StillCarriedBy the merges elsewhere that will deliver a
+	// stopped change when walked — a later merge of its repository, or one moving its modules (ADR 0299).
+	Kept           []WalkKept `json:"kept,omitempty"`
+	Withheld       []string   `json:"withheld,omitempty"`
+	StillCarriedBy []string   `json:"still_carried_by,omitempty"`
+}
+
+// WalkKept is a module a stopped walk left running its build, and where.
+type WalkKept struct {
+	Module   string   `json:"module"`
+	Machines []string `json:"machines"`
 }
 
 func (c seatController) StopWithout(walk, why, by string, without []string) (StopAnswer, error) {
@@ -290,9 +302,8 @@ func (c seatController) StopWithout(walk, why, by string, without []string) (Sto
 	}
 	var a StopAnswer
 	if json.Unmarshal(raw, &a) != nil || a.Stopped == "" {
-		// A controller from before ADR 0299 ends the walk whole and says it as text: its record names no walk
-		// that walks the others again, and every delivery on it ends stopped, as it reads.
-		return StopAnswer{Stopped: walk}, nil
+		return StopAnswer{}, fmt.Errorf("the controller's delivery-stop did not say what it left out: %s",
+			lastLine(string(raw)))
 	}
 	return a, nil
 }

modules/mesh-delivery/cmd/mesh-delivery/verbs.go

@@ -326,10 +326,19 @@ func (h *Holder) Release(id, why, by string) (string, error) {
 	if d.Walk != nil && d.Walk.Waits {
 		walk = d.Walk.ID
 	}
+	var stopped []string
+	if walk != "" {
+		stopped, _ = h.carriesStopped(walk)
+	}
 	h.mu.Unlock()
 	if walk == "" {
 		return fmt.Sprintf("%s is delivering, on %s's word", id, by), nil
 	}
+	if len(stopped) > 0 {
+		// Never the word for a walk that carries a stopped change (ADR 0299): asked to leave it out first.
+		return "", fmt.Errorf("%s is delivering on %s's word, and its walk %s is not let go yet: it carries %s, whose "+
+			"delivery was stopped, and is asked to leave it out first", id, by, walk, strings.Join(stopped, ", "))
+	}
 	if err := h.Controller.Deliver(walk, "released by "+by+": "+why); err != nil {
 		return "", fmt.Errorf("%s is delivering on %s's word, and its walk %s could not be let go yet (%v): the next "+
 			"tick asks again", id, by, walk, err)
@@ -373,30 +382,66 @@ func (h *Holder) StopWay(id, why, by, way string) (string, error) {
 		h.mu.Unlock()
 		return "", fmt.Errorf("no delivery %q", id)
 	}
+	// Its guard is read on a copy: the delivery is stopped only once its walk is (review of #221).
+	trial := *d
+	if _, err := Apply(&trial, EvStop, true, Facts{Now: h.Now(), Why: why, By: by}); err != nil {
+		h.mu.Unlock()
+		return "", err
+	}
 	var walk string
 	if d.Walk != nil && (d.Walk.State == walkBuilding || d.Walk.State == walkRolling) {
 		walk = d.Walk.ID
 	}
+	merge := d.Repository + "@" + d.MergedAs
 	var others []string
-	if walk != "" && !d.State.Final() {
-		others = h.othersOn(walk, []string{d.Repository + "@" + d.MergedAs})
+	if walk != "" {
+		others = h.othersOn(walk, []string{merge})
 	}
 	if len(others) > 0 && way == "" {
 		said := h.takenDown(d, walk, others)
 		h.mu.Unlock()
 		return "", errors.New(said)
 	}
+	h.mu.Unlock()
+
+	var did string
+	switch {
+	case walk == "":
+	case len(others) == 0 || way == wayWalk:
+		whole := why
+		if len(others) > 0 {
+			whole = fmt.Sprintf("%s was stopped, and its whole walk with it: %s", id, why)
+		}
+		if err := h.Controller.Stop(walk, whole, by); err != nil {
+			return "", fmt.Errorf("%s is not stopped: its walk %s could not be ended yet: %v — `plans stop %s --why …` "+
+				"ends it by hand", id, walk, err, walk)
+		}
+		did = fmt.Sprintf("; its walk %s is ended", walk)
+		if len(others) > 0 {
+			did = fmt.Sprintf("; its walk %s is ended whole, and every delivery on it with it: %s", walk,
+				strings.Join(others, ", "))
+		}
+	default:
+		a, err := h.Controller.StopWithout(walk, why, by, []string{merge})
+		if err != nil {
+			return "", fmt.Errorf("%s is not stopped: its walk %s could not be ended without it: %v — `way: %s` stops "+
+				"the whole walk, and `plans stop %s --why …` ends it by hand", id, walk, err, wayWalk, walk)
+		}
+		did = fmt.Sprintf("; its walk %s is ended without it, and %s", walk, againWords(a))
+	}
+
+	h.mu.Lock()
+	d = h.deliveries[id]
+	if d == nil || d.State.Final() {
+		h.mu.Unlock()
+		return fmt.Sprintf("%s ended meanwhile%s", id, did), nil
+	}
 	t, err := Apply(d, EvStop, true, Facts{Now: h.Now(), Why: why, By: by})
 	if err != nil {
 		h.mu.Unlock()
 		return "", err
 	}
 	h.moved(d, []Transition{t})
-	if walk != "" {
-		// Gathered before the group's turns, so a member after it on the same walk adds to this ask, never asks one
-		// of its own that would walk this merge again.
-		h.leave(walk, d)
-	}
 	var asks []func()
 	if g := h.groups[d.Group]; g != nil && g.Closed {
 		asks = h.groupTurns(g, h.membersOf(g))
@@ -407,31 +452,7 @@ func (h *Holder) StopWay(id, why, by, way string) (string, error) {
 	for _, a := range asks {
 		a()
 	}
-	if walk == "" {
-		return fmt.Sprintf("%s is stopped, on %s's word", id, by), nil
-	}
-	without, shared := h.leaving459(walk)
-	if !shared || way == wayWalk {
-		whole := why
-		if shared {
-			whole = fmt.Sprintf("%s was stopped, and its whole walk with it: %s", id, why)
-		}
-		if err := h.Controller.Stop(walk, whole, by); err != nil {
-			return "", fmt.Errorf("%s is stopped, and its walk %s could not be ended yet: %v — `plans stop %s --why …` "+
-				"ends it by hand", id, walk, err, walk)
-		}
-		if shared {
-			return fmt.Sprintf("%s is stopped, on %s's word; its walk %s is ended whole, and every delivery on it with "+
-				"it: %s", id, by, walk, strings.Join(others, ", ")), nil
-		}
-		return fmt.Sprintf("%s is stopped, on %s's word; its walk %s is ended", id, by, walk), nil
-	}
-	a, err := h.Controller.StopWithout(walk, why, by, without)
-	if err != nil {
-		return "", fmt.Errorf("%s is stopped, and its walk %s could not be ended without it yet: %v", id, walk, err)
-	}
-	return fmt.Sprintf("%s is stopped, on %s's word; its walk %s is ended without it, and %s", id, by, walk,
-		againWords(a)), nil
+	return fmt.Sprintf("%s is stopped, on %s's word%s", id, by, did), nil
 }
 
 // takenDown is what ending a delivery's shared walk would take down, and the two ways: for a caller holding the
@@ -451,7 +472,8 @@ func (h *Holder) takenDown(d *Delivery, walk string, others []string) string {
 	if len(contain) > 0 {
 		s += fmt.Sprintf(" These contain its merge and end with it either way: %s.", strings.Join(contain, ", "))
 	}
-	return s + fmt.Sprintf(" Choose a way: `way: %s` ends the walk and walks the others again without this merge; "+
+	return s + fmt.Sprintf(" What the walk already sent stays where it was sent; the stop's answer names it."+
+		" Choose a way: `way: %s` ends the walk and walks the others again without this merge; "+
 		"`way: %s` stops the whole walk and every delivery on it.", wayAgain, wayWalk)
 }