From aeab5b80b411fe47f7555d5bac58110709020c7a Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 19:59:43 +0200 Subject: [PATCH] mesh-delivery: a stop ends only its own change on a shared walk (hq issue 459, ADR 0299) A group stop ended the stopped member's walk, and with it every other delivery the batch put on that walk. It now asks delivery-stop to leave the stopped merges out, so the others are walked again; a person's stop on a shared walk first says what ending the walk would take down and acts only on a way. --- .../cmd/mesh-delivery/fakes_test.go | 63 +++++ .../mesh-delivery/cmd/mesh-delivery/holder.go | 110 +++++++- .../cmd/mesh-delivery/issue459_test.go | 265 ++++++++++++++++++ .../mesh-delivery/cmd/mesh-delivery/main.go | 10 +- .../mesh-delivery/cmd/mesh-delivery/model.go | 80 +++++- .../mesh-delivery/cmd/mesh-delivery/ports.go | 28 ++ .../mesh-delivery/cmd/mesh-delivery/table.go | 40 ++- .../mesh-delivery/cmd/mesh-delivery/verbs.go | 98 ++++++- 8 files changed, 659 insertions(+), 35 deletions(-) create mode 100644 modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go diff --git a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go index b140619a..a491df78 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go @@ -94,6 +94,8 @@ type fakeController struct { asked int delivers []string stops []string + // withouts are the stops that left merges out (novox/hq ADR 0299): " without ,…". + withouts []string plans int down bool // onCheck runs as a check is asked, before it is answered: what the forge held at that moment. @@ -188,6 +190,67 @@ func (c *fakeController) Stop(walk, why, by string) error { } return nil } + +// 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, "-again", waiting for its word. +func (c *fakeController) StopWithout(walk, why, by string, without []string) (StopAnswer, error) { + c.mu.Lock() + defer c.mu.Unlock() + if c.down { + return StopAnswer{}, errors.New("down") + } + c.withouts = append(c.withouts, walk+" without "+strings.Join(without, ",")) + w, known := c.walks[walk] + if !known || w.State == walkFailed || w.State == walkDone { + return StopAnswer{}, fmt.Errorf("%s is no open walk", 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 := "" + for _, s := range wd.Merges { + if named(s) && s.Repository == m.Repository && s.Carried == m.Commit { + contains = s.Commit + } + } + if contains != "" { + a.LeftOut = append(a.LeftOut, WalkLeftOut{Repository: m.Repository, Commit: m.Commit, Contains: contains}) + continue + } + m.Carried = "" + again = append(again, m) + a.WalkedAgain = append(a.WalkedAgain, m.Repository+"@"+m.Commit) + } + if len(again) > 0 { + id := walk + "-again" + 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}} + wd.WalkedAgainBy, a.WalkedAgainBy = id, id + } + wd.Stopped, wd.StoppedWhy, wd.LeftOut = by, why, a.LeftOut + w.Delivery = &wd + w.State = walkFailed + w.Note = "stopped by " + by + ": " + why + w.Revision++ + c.walks[walk] = w + return a, nil +} func (c *fakeController) Walks(walk string) (bool, []Walk, error) { c.mu.Lock() defer c.mu.Unlock() diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index 3130671b..d66cdac6 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -57,6 +57,8 @@ type Holder struct { turning map[string]*turnWant // asking are the walks whose word is being asked right now, outside the lock: not asked twice meanwhile. 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 } // unmatched is a walk no delivery is known for yet: its merge not heard, or merged before this owner held. @@ -642,6 +644,15 @@ func (h *Holder) linkWalk(d *Delivery, w Walk, carried string) { d.LetGo, d.LetGoWhy = time.Time{}, "" // a new walk waits for a word of its own } d.Walk = w.Seen() + // Stopped without some of its merges (novox/hq ADR 0299): this delivery's merge was left out, maybe for + // containing a stopped one, or is walked again by the batch the record names. + if w.Delivery != nil && (w.Delivery.WalkedAgainBy != "" || len(w.Delivery.LeftOut) > 0) { + if o, out := w.leftOut(d.Repository, d.MergedAs); out { + d.Walk.Contains = o.Contains + } else { + d.Walk.WalkedAgainBy = w.Delivery.WalkedAgainBy + } + } if carried != "" { d.AnsweredBy = carried } @@ -1807,7 +1818,13 @@ func mayCarry(c, d *Delivery, module string) bool { c.Plan != nil && slices.Contains(c.Plan.Moved, module) } -// stopInGroup stops a member after one that failed, and its walk if it has one open. +// stopInGroup stops a member after one that failed, and leaves its merge out of its walk if it has one open. +// +// **Only its own change** (novox/hq issue 459, ADR 0299): one walk carries every merge of its batch, so ending the +// member's walk ended every other delivery on it, which nobody stopped. Its merge is left out instead: the +// controller ends the walk and walks the other merges again without it, and without each later merge of its +// repository, which contains it. Every member a pass stops on one walk is gathered first, and the walk is asked +// once, by the first member's ask, after the pass: a member is never walked again because its stop came second. func (h *Holder) stopInGroup(d *Delivery, why string) func() { if d == nil || d.State.Final() { return nil @@ -1822,11 +1839,98 @@ func (h *Holder) stopInGroup(d *Delivery, why string) func() { 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 + } return func() { - if err := h.Controller.Stop(walk, why, byOwner); err != nil { - h.Logf("[mesh-delivery] the walk %s could not be stopped: %v", walk, err) + without, shared := h.leaving459(walk) + if !shared { + if err := h.Controller.Stop(walk, why, byOwner); err != nil { + h.Logf("[mesh-delivery] the walk %s could not be stopped: %v", walk, err) + } + 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)) + } +} + +// 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 { + if h.leaving == nil { + h.leaving = map[string][]string{} + } + first := len(h.leaving[walk]) == 0 + if m := d.Repository + "@" + d.MergedAs; d.MergedAs != "" && !containsString(h.leaving[walk], m) { + h.leaving[walk] = append(h.leaving[walk], m) + } + return first +} + +// leaving459 takes the merges gathered to leave out of a walk, and whether the walk carries anything else: another +// merge it names, or a delivery not ended that follows it. Taken under the lock, outside any. +func (h *Holder) leaving459(walk string) ([]string, bool) { + h.mu.Lock() + defer h.mu.Unlock() + without := h.leaving[walk] + delete(h.leaving, walk) + return without, len(h.othersOn(walk, without)) > 0 +} + +// othersOn are what a walk carries besides the merges named: each delivery not ended that follows it, by id, and +// each merge it names that no such delivery holds, as `@`. +func (h *Holder) othersOn(walk string, without []string) []string { + out := map[string]bool{} + held := map[string]bool{} + for _, d := range h.deliveries { + if d.Walk == nil || d.Walk.ID != walk || d.MergedAs == "" { + continue + } + m := d.Repository + "@" + d.MergedAs + held[strings.ToLower(m)] = true + if !d.State.Final() && !containsString(without, m) { + out[d.ID] = true } } + if s, known := h.walks[walk]; known && s.walk.Delivery != nil { + for _, m := range s.walk.Delivery.Merges { + key := m.Repository + "@" + m.Commit + if !held[strings.ToLower(key)] && !slices.ContainsFunc(without, func(w string) bool { return strings.EqualFold(w, key) }) { + out[key] = true + } + } + } + keys := make([]string, 0, len(out)) + for k := range out { + keys = append(keys, k) + } + slices.Sort(keys) + return keys +} + +// againWords is what a stop that left merges out did, in a person's words. +func againWords(a StopAnswer) string { + var left []string + for _, o := range a.LeftOut { + s := o.Repository + "@" + shortOf(o.Commit, 12) + if o.Contains != "" { + s += " (contains " + shortOf(o.Contains, 12) + ")" + } + left = append(left, s) + } + switch { + case a.WalkedAgainBy != "": + return 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" + } + return "the controller ended it whole: it named no walk that walks its other merges again" } // GroupState is a group's state, derived from its members and its composed check — never set apart (novox/hq diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go new file mode 100644 index 00000000..98dc5e62 --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue459_test.go @@ -0,0 +1,265 @@ +package main + +import ( + "fmt" + "strings" + "testing" + "time" +) + +// novox/hq issue 459, ADR 0299: since the merge window (ADR 0276) one walk carries every merge of its batch. A group +// stop asked the controller to end the stopped member's walk, and with it every other delivery on it, which no one +// stopped. Now a group stop asks `delivery-stop` with the stopped members' merges to leave out: the walk ends, and +// the controller walks its other merges again in a fresh batch; a later merge of a stopped merge's repository +// contains it and ends with it, named. A person's stop of a delivery on a shared walk answers first with what +// ending the walk would take down, and acts only on a way given: `again` or `walk`. + +// shape459 is a world of a delivery group feat/g (its members in the repositories group names, in that order) and +// lone deliveries (the repositories alone names), each checked and merged. The group's first member is on a walk of +// its own, plan-a; every other merge is carried by one walk, plan-2, waiting for its word. +type shape459 struct { + *world + ids, heads, merges map[string]string // by repository +} + +func open459(t *testing.T, group, alone []string) *shape459 { + t.Helper() + w := newWorld(t) + s := &shape459{world: w, ids: map[string]string{}, heads: map[string]string{}, merges: map[string]string{}} + n := 0 + add := func(repo, branch string) { + n++ + head := fmt.Sprintf("%02d%s", n, strings.Repeat("h", 14)) + s.ids[repo], s.heads[repo] = IDOf(repo, head), head + s.merges[repo] = fmt.Sprintf("%02d%s", n, strings.Repeat("m", 10)) + w.h.PullUpdated(pr(repo, n, head, branch, "x")) + w.h.Checked(verdict(repo, n, head, "pass")) + } + for _, r := range group { + add(r, "feat/g") + } + for _, r := range alone { + add(r, "fix/"+r[strings.LastIndex(r, "/")+1:]) + } + w.settleAll() + w.h.Checked(CheckedEvent{Group: "feat/g", ID: "check-1", Verdict: "pass", Summary: "compose"}) + w.settleAll() + for i, r := range append(append([]string{}, group...), alone...) { + w.h.PullMerged(merged(r, i+1, s.heads[r], s.merges[r])) + } + first := aWalk("plan-a", group[0], s.merges[group[0]], true) + first.Delivery.Merges = []WalkMerge{{Repository: group[0], Commit: s.merges[group[0]]}} + w.h.WalkMoved(w.ctl.put(first)) + two := aWalk("plan-2", alone[0], s.merges[alone[0]], true) + two.Created = two.Created.Add(time.Minute) + for _, r := range append(append([]string{}, group[1:]...), alone...) { + two.Delivery.Merges = append(two.Delivery.Merges, WalkMerge{Repository: r, Commit: s.merges[r]}) + } + w.h.WalkMoved(w.ctl.put(two)) + w.settleAll() + return s +} + +// failFirst fails the group's first member's walk: the members after it are stopped by their group. +func (s *shape459) failFirst() { + a := s.ctl.walk("plan-a") + a.State, a.Note = walkFailed, "the first member failed its gate on anchor" + s.h.WalkMoved(s.ctl.put(a)) + s.settleAll() +} + +// heard is the controller's records of the stopped walk and the walk that walks its merges again, as plan-moved says +// them, and then a walk done. +func (s *shape459) heard(walks ...string) { + for _, id := range walks { + s.h.WalkMoved(s.ctl.walk(id)) + } + s.settleAll() +} + +func (s *shape459) done(walk string) { + r := s.ctl.walk(walk) + sent := s.now + for m := range r.Modules { + r.Modules[m] = &WalkModule{State: "built", SentAt: &sent} + } + r.State = walkDone + s.h.WalkMoved(s.ctl.put(r)) + s.settleAll() +} + +func lastWhy(d *Delivery) string { + if len(d.Transitions) == 0 { + return "" + } + return d.Transitions[len(d.Transitions)-1].Why +} + +// A group member stopped on a walk it shares with a lone delivery: the walk is stopped without the member's merge, +// never whole, and the lone delivery follows the walk that walks its merge again and is delivered by it. +func TestAGroupStopWalksTheOtherDeliveriesAgainWithoutIt(t *testing.T) { + s := open459(t, []string{"novox/a", "novox/b"}, []string{"novox/z"}) + if containsString(s.ctl.delivers, "plan-2") { + t.Fatalf("plan-2 was let go before b's turn: %v", s.ctl.delivers) + } + s.failFirst() + if b := s.delivery(s.ids["novox/b"]); b.State != Stopped || !strings.Contains(lastWhy(b), s.ids["novox/a"]) { + t.Fatalf("b is %s (%q); want it stopped by its group, naming a", b.State, lastWhy(b)) + } + if containsString(s.ctl.stops, "plan-2") { + t.Fatalf("plan-2 was ended whole %v, and with it z, which is in no group and was not stopped", s.ctl.stops) + } + if want := "plan-2 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", "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) + } + s.done("plan-2-again") + if z, b := s.state(s.ids["novox/z"]), s.state(s.ids["novox/b"]); z != Delivered || b != Stopped { + t.Fatalf("once plan-2-again is done, z is %s and b %s; want z delivered and b stopped", z, b) + } +} + +// Two members after the failed one, both on the shared walk: one ask leaves both out, so neither is walked again +// because its own stop came second. +func TestAGroupStopLeavesOutEveryMemberItStopsOnOneWalk(t *testing.T) { + s := open459(t, []string{"novox/a", "novox/b", "novox/c"}, []string{"novox/z"}) + s.failFirst() + if len(s.ctl.withouts) != 1 || !strings.Contains(s.ctl.withouts[0], "novox/b@"+s.merges["novox/b"]) || + !strings.Contains(s.ctl.withouts[0], "novox/c@"+s.merges["novox/c"]) { + t.Fatalf("the controller was asked %v; want one stop of plan-2 leaving out b and c", s.ctl.withouts) + } + s.heard("plan-2", "plan-2-again") + again := s.ctl.walk("plan-2-again") + if len(again.Delivery.Merges) != 1 || again.Delivery.Merges[0].Repository != "novox/z" { + t.Fatalf("plan-2-again walks %+v; want z's merge alone", again.Delivery.Merges) + } + for _, r := range []string{"novox/b", "novox/c"} { + if st := s.state(s.ids[r]); st != Stopped { + t.Fatalf("%s is %s", r, st) + } + } +} + +// A lone delivery whose merge is a later merge of the stopped member's repository contains the member's merge: it +// is left out and ends stopped, naming the merge it contains; the other lone delivery is walked again. +func TestAGroupStopEndsAMergeThatContainsTheStoppedOne(t *testing.T) { + s := open459(t, []string{"novox/a", "novox/b"}, []string{"novox/z", "novox/b2"}) + // b2 is a later merge of b's repository: the walk carries it for b. + s.retarget("novox/b2", "novox/b") + s.failFirst() + s.heard("plan-2", "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") { + t.Fatalf("b2 is %s (%q); want it stopped, naming b's merge it contains", b2.State, lastWhy(b2)) + } + if z := s.delivery(s.ids["novox/z"]); z.Walk == nil || z.Walk.ID != "plan-2-again" || z.State.Final() { + t.Fatalf("z is %s on %+v; want it following plan-2-again", z.State, z.Walk) + } +} + +// retarget makes the delivery of repo `later` a later merge of repository `of`: same repository, carried by plan-2 +// for the merge of `of`. +func (s *shape459) retarget(later, of string) { + s.t.Helper() + s.h.mu.Lock() + d := s.h.deliveries[s.ids[later]] + d.Repository, d.MergedAt = of, d.MergedAt.Add(time.Minute) + s.h.mu.Unlock() + two := s.ctl.walk("plan-2") + merges := []WalkMerge{} + for _, m := range two.Delivery.Merges { + switch m.Repository { + case of: + m.Carried = s.merges[later] + case later: + m.Repository = of + } + merges = append(merges, m) + } + wd := *two.Delivery + wd.Merges = merges + two.Delivery = &wd + s.h.WalkMoved(s.ctl.put(two)) + s.settleAll() +} + +// shared is two lone deliveries, y and z, let go together on one walk, plan-2. +func shared459(t *testing.T) *shape459 { + t.Helper() + s := open459(t, []string{"novox/a"}, []string{"novox/y", "novox/z"}) + for _, r := range []string{"novox/y", "novox/z"} { + if st := s.state(s.ids[r]); st != Delivering { + t.Fatalf("%s is %s, let go %v; want both delivering on plan-2", r, st, s.ctl.delivers) + } + } + return s +} + +// A person's stop of y, which shares its walk with z, does nothing at first: it names z and the two ways. +func TestAPersonsStopOfASharedWalkAsksFirst(t *testing.T) { + s := shared459(t) + said, err := s.h.StopWay(s.ids["novox/y"], "not wanted after all", "jochen", "") + if err == nil { + t.Fatalf("the stop acted at once: %q", said) + } + for _, want := range []string{s.ids["novox/z"], "plan-2", "again", "walk", "not stopped"} { + if !strings.Contains(err.Error(), want) { + t.Fatalf("the stop said %q; want it to name %q", err, want) + } + } + if st := s.state(s.ids["novox/y"]); st != Delivering || len(s.ctl.stops)+len(s.ctl.withouts) != 0 { + t.Fatalf("y is %s, the controller was asked %v %v; want nothing done", st, s.ctl.stops, s.ctl.withouts) + } + if _, err := s.h.StopWay(s.ids["novox/y"], "x", "jochen", "sideways"); err == nil { + t.Fatal("a way that is neither again nor walk was taken") + } +} + +// With `again`, y is stopped, the walk is stopped without y's merge, and z goes back to published and follows the +// walk that walks it again. +func TestAPersonsStopAgainWalksTheOthersAgain(t *testing.T) { + s := shared459(t) + said, err := s.h.StopWay(s.ids["novox/y"], "not wanted after all", "jochen", "again") + if err != nil { + t.Fatal(err) + } + if !strings.Contains(said, "plan-2-again") || !strings.Contains(said, "novox/z@") { + t.Fatalf("the stop said %q; want it to name the walk that walks z again", said) + } + if want := "plan-2 without novox/y@" + s.merges["novox/y"]; !containsString(s.ctl.withouts, want) || + containsString(s.ctl.stops, "plan-2") { + t.Fatalf("the controller was asked %v, ended whole %v; want %q", s.ctl.withouts, s.ctl.stops, want) + } + s.heard("plan-2") + if z := s.delivery(s.ids["novox/z"]); z.State != Published { + 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") + 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) + } + if st := s.state(s.ids["novox/y"]); st != Stopped { + t.Fatalf("y is %s", st) + } +} + +// With `walk`, the whole walk is stopped, and z ends stopped, naming y's stop. +func TestAPersonsStopOfTheWalkStopsEveryDelivery(t *testing.T) { + s := shared459(t) + if _, err := s.h.StopWay(s.ids["novox/y"], "not wanted after all", "jochen", "walk"); err != nil { + t.Fatal(err) + } + if !containsString(s.ctl.stops, "plan-2") || len(s.ctl.withouts) != 0 { + t.Fatalf("the controller was asked %v %v; want plan-2 ended whole", s.ctl.stops, s.ctl.withouts) + } + s.heard("plan-2") + z := s.delivery(s.ids["novox/z"]) + if z.State != Stopped || !strings.Contains(lastWhy(z), s.ids["novox/y"]) { + t.Fatalf("z is %s (%q); want it stopped, naming y's stop", z.State, lastWhy(z)) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/main.go b/modules/mesh-delivery/cmd/mesh-delivery/main.go index 6fe80a48..92be1bb8 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/main.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/main.go @@ -293,14 +293,18 @@ func tools(h *Holder, l *listening) []stdio.Tool { }}, {Name: seat + "stop", Description: "Stop a delivery that is not final, with why: its walk is ended through the controller, and " + - "in a delivering group every member after it is stopped too, naming it.", + "in a delivering group every member after it is stopped too, naming it. When its walk carries other " + + "deliveries, it first answers what ending the walk would take down, and acts only on a way: `again` " + + "ends the walk and walks the others again without this merge, `walk` stops the whole walk (novox/hq ADR 0299).", Input: map[string]any{"type": "object", "properties": map[string]any{"id": str("the delivery's id"), - "why": str("why, kept with the transition")}, "required": []string{"id", "why"}}, + "why": str("why, kept with the transition"), + "way": str("on a walk shared with other deliveries: again (the others walked again without it) or walk " + + "(the whole walk stopped)")}, "required": []string{"id", "why"}}, Run: func(a map[string]any) (any, error) { if err := need(a, "id", "why"); err != nil { return nil, err } - return h.Stop(strArg(a, "id"), strArg(a, "why"), aPerson) + return h.StopWay(strArg(a, "id"), strArg(a, "why"), aPerson, strArg(a, "way")) }}, {Name: seat + "close", Description: "Healer H2's verb: for a delivery held past its bound, read its walk again and take the " + diff --git a/modules/mesh-delivery/cmd/mesh-delivery/model.go b/modules/mesh-delivery/cmd/mesh-delivery/model.go index 5c9617b9..0fb09377 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/model.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/model.go @@ -383,6 +383,11 @@ type WalkDelivery struct { // TakenOverBy is, on a walk folded before it started, the walk that took it over and answers every merge // it answered. TakenOverBy string `json:"taken_over_by,omitempty"` + // WalkedAgainBy is, on a walk stopped without some of its merges, the batch that walks its other merges again, + // and LeftOut the merges it left out (novox/hq ADR 0299): a delivery of a merge left out ends stopped with it, + // one of a merge walked again follows that batch's walk. + WalkedAgainBy string `json:"walked_again_by,omitempty"` + LeftOut []WalkLeftOut `json:"left_out,omitempty"` // Batch is set while the record is a batch, not yet a walk: its state `assembling` or `queued`. A batch, once // cut, is the walk under the same id: its next record is `building`. A merge this owner reads in a batch is // not held as unwalked while the batch is heard of. @@ -398,6 +403,27 @@ type WalkMerge struct { Carried string `json:"carried,omitempty"` } +// WalkLeftOut is a merge a stop left out (novox/hq ADR 0299): a merge stopped, or a later merge of its +// repository's branch that contains it, Contains naming the stopped merge. +type WalkLeftOut struct { + Repository string `json:"repository"` + Commit string `json:"commit"` + Contains string `json:"contains,omitempty"` +} + +// leftOut is the merge of a repository a stop left out, when it did. +func (w Walk) leftOut(repository, merge string) (WalkLeftOut, bool) { + if w.Delivery == nil || merge == "" { + return WalkLeftOut{}, false + } + for _, o := range w.Delivery.LeftOut { + if strings.EqualFold(o.Repository, repository) && sameCommit(o.Commit, merge) { + return o, true + } + } + return WalkLeftOut{}, false +} + // WalkBatch is a batch's window: when it closes unless another merge arrives, when it closes at the latest, // and, once closed, the walk it waits behind. type WalkBatch struct { @@ -454,18 +480,24 @@ func (w Walk) carriedFor(repository, merge string) (string, bool) { // WalkSeen is what a delivery keeps of its walk. type WalkSeen struct { - ID string `json:"id"` - Commit string `json:"commit"` - State string `json:"state"` - Note string `json:"note,omitempty"` - Revision int64 `json:"revision"` - Waits bool `json:"waits,omitempty"` - WaitedFor string `json:"waited_for,omitempty"` - GoBy string `json:"go_by,omitempty"` - StoppedBy string `json:"stopped_by,omitempty"` - Tier int `json:"tier"` - Tiers int `json:"tiers"` - Updated time.Time `json:"updated"` + ID string `json:"id"` + Commit string `json:"commit"` + State string `json:"state"` + Note string `json:"note,omitempty"` + Revision int64 `json:"revision"` + Waits bool `json:"waits,omitempty"` + WaitedFor string `json:"waited_for,omitempty"` + GoBy string `json:"go_by,omitempty"` + StoppedBy string `json:"stopped_by,omitempty"` + // StoppedWhy is why it was stopped; WalkedAgainBy, on a walk stopped without some merges, the batch that walks + // this delivery's merge again; Contains, when this delivery's merge was left out for containing a stopped + // merge, that merge (novox/hq ADR 0299). + StoppedWhy string `json:"stopped_why,omitempty"` + WalkedAgainBy string `json:"walked_again_by,omitempty"` + Contains string `json:"contains,omitempty"` + Tier int `json:"tier"` + Tiers int `json:"tiers"` + Updated time.Time `json:"updated"` // Branch is the branch the walk's commit was merged into; empty in a record kept before it was read. Branch string `json:"branch,omitempty"` // Created is when the controller made the walk: a delivery follows the newest walk that answers it. @@ -487,7 +519,7 @@ func (w Walk) Seen() *WalkSeen { s.WaitedFor = d.Awaits s.Waits = d.Awaits != "" && d.Go == nil s.GoBy = d.By - s.StoppedBy = d.Stopped + s.StoppedBy, s.StoppedWhy = d.Stopped, d.StoppedWhy } s.Branch = w.Branch s.Created = w.Created @@ -511,6 +543,28 @@ func (d *Delivery) endedUnstarted() bool { return d.State == Published && d.Walk != nil && d.Walk.State == walkDone && !d.Walk.Started() } +// walkStopped is whether the walk it follows was stopped through this owner, and walkedAgain whether its merge is +// walked again by another walk since (novox/hq ADR 0299). +func (d *Delivery) walkStopped() bool { + return d.Walk != nil && d.Walk.State == walkFailed && d.Walk.StoppedBy != "" +} + +func (d *Delivery) walkedAgain() bool { return d.walkStopped() && d.Walk.WalkedAgainBy != "" } + +// stoppedWords is why a delivery ends with the walk it follows stopped: the stop, and the stopped merge its own +// contains when it was left out for that. +func (d *Delivery) stoppedWords() string { + s := fmt.Sprintf("its walk %s was stopped by %s", d.Walk.ID, d.Walk.StoppedBy) + if d.Walk.StoppedWhy != "" { + s += ": " + d.Walk.StoppedWhy + } + if d.Walk.Contains != "" { + s += fmt.Sprintf("; its merge contains the stopped merge %s, so it cannot be walked without delivering it, "+ + "and was left out with it (novox/hq ADR 0299)", shortOf(d.Walk.Contains, 12)) + } + return s +} + // walkMovedItsModules is whether the walk it follows built every module its plan moves, and whether that is known: // from the modules the walk's record names, or, for a record kept before they were, from a walk of no tier, which // moved nothing. Unknown when its plan is not known. diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ports.go b/modules/mesh-delivery/cmd/mesh-delivery/ports.go index a9819fc7..072220ce 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ports.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ports.go @@ -61,6 +61,10 @@ type Controller interface { Deliver(walk, why string) error // Stop ends a walk, said as stopped by whom. Stop(walk, why, by string) error + // StopWithout ends a walk and walks its other merges again without the merges named, each + // `@`, and without each later merge of their repository, which contains one (novox/hq + // ADR 0299). + StopWithout(walk, why, by string, without []string) (StopAnswer, error) // Walks are the walks the controller keeps — one, given its id — and whether this seat has a holder on record. Walks(walk string) (bool, []Walk, error) } @@ -269,6 +273,30 @@ func (c seatController) Stop(walk, why, by string) error { return err } +// StopAnswer is what the controller's `delivery-stop` with merges to leave out answers (novox/hq ADR 0299): the walk +// stopped, the batch that walks its other merges again with those merges, and every merge left out. +type StopAnswer struct { + Stopped string `json:"stopped"` + WalkedAgainBy string `json:"walked_again_by,omitempty"` + WalkedAgain []string `json:"walked_again,omitempty"` + LeftOut []WalkLeftOut `json:"left_out,omitempty"` +} + +func (c seatController) StopWithout(walk, why, by string, without []string) (StopAnswer, error) { + raw, err := c.verb("delivery-stop", map[string]any{"plan": walk, "why": why, "by": by, + "without": strings.Join(without, ",")}) + if err != nil { + return StopAnswer{}, err + } + 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 a, nil +} + func (c seatController) Walks(walk string) (bool, []Walk, error) { args := map[string]any{} if walk != "" { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/table.go b/modules/mesh-delivery/cmd/mesh-delivery/table.go index 76005611..707bd563 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/table.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/table.go @@ -84,7 +84,7 @@ const ( EvRetire Event = "retire" // a person's word that a held delivery is history, never to be walked EvCovered Event = "covered" // a later merge into the same trunk delivered every module it moves 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: it waits for a walk of its own commit + 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 ) // Facts are what a guard reads beyond the delivery itself: the moment, and who asks, for an act. @@ -259,6 +259,9 @@ var Table = []Row{ if d.Walk != nil && d.Walk.State == walkFailed && d.carriedLater() { return errors.New("its walk, which carried a later commit for it, failed: it waits for a walk of its own commit") } + if d.walkStopped() { + return errors.New("its walk was stopped: it waits for the walk that walks its merge again, or ends stopped") + } return unless(d.Walk != nil && d.Walk.Started(), "its walk waits") }}, // **A walk that ended before its word settles its delivery** (novox/hq issue 474): a batch's walk the @@ -284,11 +287,21 @@ var Table = []Row{ }}, {From: []State{Published}, Event: EvStop, To: Stopped, Guard: "its walk ended done before its word without building every module it moves, and no later merge that " + - "contains it delivered them: no start can come, and nothing delivered it (novox/hq issue 474)", + "contains it delivered them: no start can come, and nothing delivered it (novox/hq issue 474); or the walk " + + "it waits on was stopped through this owner and walks its merge again nowhere — left out, or the walk " + + "ended whole (novox/hq ADR 0299)", Holds: func(d *Delivery, f Facts) error { + if d.walkStopped() && !d.walkedAgain() { + return nil + } return unless(d.endedUnstarted() && d.Stranded != "", "its walk has not ended unstarted, or something may cover it") }, - Says: func(d *Delivery) string { return d.Stranded }}, + Says: func(d *Delivery) string { + if d.walkStopped() { + return d.stoppedWords() + } + return d.Stranded + }}, {From: []State{Held}, Event: EvGo, To: Delivering, Guard: "its walk started on a word that was not this owner's: the controller's own path, which waits for " + "nobody, or a person's `plans go`", @@ -332,10 +345,15 @@ var Table = []Row{ return "covered: a later merge that contains it was delivered: " + d.Carrier }}, {From: []State{Delivering}, Event: EvStop, To: Stopped, - Guard: "its walk was stopped through this owner", + 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)", Holds: func(d *Delivery, f Facts) error { - return unless(d.Walk != nil && d.Walk.State == walkFailed && d.Walk.StoppedBy != "", "its walk was not stopped") - }}, + if d.walkedAgain() { + return fmt.Errorf("its walk was stopped without it, and %s walks its merge again", d.Walk.WalkedAgainBy) + } + return unless(d.walkStopped(), "its walk was not stopped") + }, + Says: func(d *Delivery) string { return d.stoppedWords() }}, {From: []State{Delivering}, Event: EvFailed, To: Failed, Guard: "its walk failed, carrying its own commit: a gate on a first machine (what it carried put back), a build, a machine", Holds: func(d *Delivery, f Facts) error { @@ -344,12 +362,20 @@ var Table = []Row{ }}, {From: []State{Delivering}, Event: EvRewalk, To: Published, Guard: "its walk failed carrying a later merge of its branch for it: a failed walk marks no merge delivered, and this " + - "merge waits, published, for a walk of its own commit (novox/hq ADR 0276 decision 3)", + "merge waits, published, for a walk of its own commit (novox/hq ADR 0276 decision 3); or its walk was stopped " + + "for another merge, and its merge is walked again without that one (novox/hq ADR 0299)", Holds: func(d *Delivery, f Facts) error { + if d.walkedAgain() { + return nil + } return unless(d.Walk != nil && d.Walk.State == walkFailed && d.Walk.StoppedBy == "" && d.carriedLater(), "its walk did not fail, or carried its own commit") }, Says: func(d *Delivery) string { + if d.walkedAgain() { + return fmt.Sprintf("its walk %s was stopped for another merge (%s): %s walks its merge again without that one", + d.Walk.ID, d.Walk.StoppedWhy, d.Walk.WalkedAgainBy) + } return fmt.Sprintf("its walk %s, which carried %s for it, failed: it waits for a walk of its own commit", d.Walk.ID, shortOf(d.AnsweredBy, 12)) }}, diff --git a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go index fbee6df1..14fe3209 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go @@ -346,24 +346,56 @@ func (h *Holder) Release(id, why, by string) (string, error) { return fmt.Sprintf("%s is delivering on %s's word; its walk %s is let go", id, by, walk), nil } -// Stop is the `stop` verb: a delivery stopped, its walk ended through the controller, and in a group every +// Stop is the `stop` verb with no way given: see StopWay. +func (h *Holder) Stop(id, why, by string) (string, error) { return h.StopWay(id, why, by, "") } + +// The ways a person's stop of a delivery on a shared walk takes (novox/hq ADR 0299). +const ( + wayAgain = "again" // the walk ended, its other merges walked again without this one + wayWalk = "walk" // the whole walk stopped, every delivery on it with it +) + +// StopWay is the `stop` verb: a delivery stopped, its walk ended through the controller, and in a group every // member after it stopped too. -func (h *Holder) Stop(id, why, by string) (string, error) { +// +// **A stop never takes other deliveries down quietly** (novox/hq issue 459, ADR 0299): one walk carries every merge +// of its batch. When the delivery's open walk carries anything else, the stop does nothing at first: it says what +// ending the walk would take down — each other delivery, and which of them contain this merge and end either way — +// and the two ways, given with `way`: `again` ends the walk and walks the others again without this merge and the +// merges that contain it; `walk` stops the whole walk, and every delivery on it ends stopped, naming this stop. +func (h *Holder) StopWay(id, why, by, way string) (string, error) { + if way != "" && way != wayAgain && way != wayWalk { + return "", fmt.Errorf("way %q: %q or %q", way, wayAgain, wayWalk) + } h.mu.Lock() d := h.deliveries[id] if d == nil { h.mu.Unlock() return "", fmt.Errorf("no delivery %q", id) } + var walk string + if d.Walk != nil && (d.Walk.State == walkBuilding || d.Walk.State == walkRolling) { + walk = d.Walk.ID + } + var others []string + if walk != "" && !d.State.Final() { + others = h.othersOn(walk, []string{d.Repository + "@" + d.MergedAs}) + } + if len(others) > 0 && way == "" { + said := h.takenDown(d, walk, others) + h.mu.Unlock() + return "", errors.New(said) + } 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}) - var walk string - if d.Walk != nil && (d.Walk.State == walkBuilding || d.Walk.State == walkRolling) { - walk = d.Walk.ID + 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 { @@ -378,11 +410,59 @@ func (h *Holder) Stop(id, why, by string) (string, error) { if walk == "" { return fmt.Sprintf("%s is stopped, on %s's word", id, by), nil } - if err := h.Controller.Stop(walk, why, 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) + 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 } - 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 +} + +// takenDown is what ending a delivery's shared walk would take down, and the two ways: for a caller holding the +// lock (ADR 0299). +func (h *Holder) takenDown(d *Delivery, walk string, others []string) string { + var ride, contain []string + for _, o := range others { + od := h.deliveries[o] + if od != nil && containsMerge(od, d) { + contain = append(contain, o) + continue + } + ride = append(ride, o) + } + s := fmt.Sprintf("%s is not stopped: its walk %s carries other changes, and ending the walk would take them down: %s.", + d.ID, walk, orNone(strings.Join(ride, ", "))) + 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; "+ + "`way: %s` stops the whole walk and every delivery on it.", wayAgain, wayWalk) +} + +// containsMerge is whether o is a later merge of d's repository and branch, which contains d's merge on a linear +// trunk: merged after it, or the merge the walk carries for it. +func containsMerge(o, d *Delivery) bool { + if o.ID == d.ID || !strings.EqualFold(o.Repository, d.Repository) || !strings.EqualFold(o.Base, d.Base) || o.MergedAs == "" { + return false + } + return o.MergedAt.After(d.MergedAt) || d.AnsweredBy != "" && sameCommit(d.AnsweredBy, o.MergedAs) && + !sameCommit(d.AnsweredBy, d.MergedAs) } // Close is healer H2's verb: for a delivery past its bound, the walk's record is read again and the