From e8cb2ee470c2a081f56afe5c8d720101d2d3d3de Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 23:53:52 +0200 Subject: [PATCH 1/2] Wait for the batch that took a folded walk over, and let a delivery superseded by its walk follow that walk (issue 1786) --- .../cmd/mesh-delivery-gate/holder.go | 11 ++- .../cmd/mesh-delivery-gate/issue1786_test.go | 99 +++++++++++++++++++ .../cmd/mesh-delivery-gate/table.go | 30 ++++++ .../cmd/mesh-delivery-gate/table_test.go | 15 ++- 4 files changed, 150 insertions(+), 5 deletions(-) create mode 100644 modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1786_test.go diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go index 9b6d8135..b431eef3 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go +++ b/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 diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1786_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1786_test.go new file mode 100644 index 00000000..a7feadde --- /dev/null +++ b/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) + } +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table.go index b8ed6d08..cd5479e4 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table.go +++ b/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) +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table_test.go index 573e4935..42db6d43 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table_test.go +++ b/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. From c4176107f287494c15336d131fe8243d52cbe188 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 22 Oct 2026 00:01:22 +0200 Subject: [PATCH 2/2] Keep a delivery superseded by its walk final, and heal it as delivered by the walk that took its walk over (issue 1786) --- .../cmd/mesh-delivery-gate/effects.go | 3 ++ .../cmd/mesh-delivery-gate/fakes_test.go | 13 ++++--- .../cmd/mesh-delivery-gate/holder.go | 24 ++++++++++--- .../cmd/mesh-delivery-gate/issue1786_test.go | 35 ++++++++++++++++--- .../cmd/mesh-delivery-gate/model.go | 15 +++++++- .../cmd/mesh-delivery-gate/table.go | 30 ---------------- .../cmd/mesh-delivery-gate/table_test.go | 15 ++------ 7 files changed, 78 insertions(+), 57 deletions(-) diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/effects.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/effects.go index c5f0aeb4..e765003a 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/effects.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/effects.go @@ -154,6 +154,9 @@ func statusOf(d *Delivery) (string, string) { } return "failure", clip("failed: "+why, 140) case Superseded: + if d.coveredStop() { + return "success", clip(d.coveredStopSaid(), 140) // its walk's taker delivered it (issue 1786) + } return "warning", clip("superseded: "+why, 140) case Stopped: if d.coveredStop() { diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/fakes_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/fakes_test.go index 61de32c0..d7c47f8e 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/fakes_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/fakes_test.go @@ -601,10 +601,11 @@ type world struct { source *fakeSource owed *fakeOwed // asked is the key of the ask a test answered. - asked string - now time.Time - said []map[string]any - mu sync.Mutex + asked string + now time.Time + logbuf strings.Builder + said []map[string]any + mu sync.Mutex } func newWorld(t *testing.T) *world { @@ -615,7 +616,7 @@ func newWorldOver(t *testing.T, store *memStore, ctl *fakeController, forge *fak w := &world{t: t, store: store, ctl: ctl, forge: forge, now: now} w.h = &Holder{Store: store, Controller: ctl, Forge: forge, Now: func() time.Time { w.mu.Lock(); defer w.mu.Unlock(); return w.now }, - Logf: func(format string, a ...any) { t.Logf(format, a...) }, + Logf: func(format string, a ...any) { t.Logf(format, a...); fmt.Fprintf(&w.logbuf, format+"\n", a...) }, Emit: func(event string, body any) error { w.mu.Lock() defer w.mu.Unlock() @@ -719,3 +720,5 @@ func (f *fakeForge) Comment(owner, repo string, number int, body string) error { f.calls = append(f.calls, "comment "+body) return nil } + +func (w *world) logs() string { return w.logbuf.String() } diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go index b431eef3..35599fb2 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go @@ -1396,7 +1396,23 @@ 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.Walk == nil || d.Walk.TakenOverBy == "" || d.State.Final() && !d.supersededByItsWalk() { + if d.Walk == nil || d.Walk.TakenOverBy == "" { + continue + } + if d.State.Final() { + // Ended superseded by its walk before the walk that took it over was heard to answer (issue 1786): the + // state stays final, as a covered superseded member's does (issue 442), and the end of that walk is + // kept as DeliveredBy, which the group and the views read. Never logged, never changed otherwise. + if d.supersededByItsWalk() && d.DeliveredBy == "" { + if s, known := h.walks[d.Walk.TakenOverBy]; known { + if _, ok := walkAnswers(s.walk, d); ok && s.walk.State == walkDone { + d.DeliveredBy = fmt.Sprintf("the walk %s, which took its walk %s over, ended done", s.walk.ID, d.Walk.ID) + h.tellCovered(d, now) + } + } else if a := h.askWalk(d.Walk.TakenOverBy, "which took a delivery's walk over", now); a != nil { + asks = append(asks, a) + } + } continue } if s, known := h.walks[d.Walk.TakenOverBy]; known { @@ -1411,9 +1427,7 @@ func (h *Holder) plan() []func() { // 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) - if !d.State.Final() { - d.Walk.TakenOverBy = "" - } + d.Walk.TakenOverBy = "" } h.settleAndKeep(d) continue @@ -2188,7 +2202,7 @@ func (h *Holder) planGroup(g *Group, members []*Delivery, now time.Time) []func( // moves — and its group counts it as reached (novox/hq issue 442). if g.Closed { for _, d := range members { - if d.State != Superseded || d.CoveredBy != "" { + if d.State != Superseded || d.CoveredBy != "" || d.DeliveredBy != "" { continue } var ask []string diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1786_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1786_test.go index a7feadde..79a4f60c 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1786_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1786_test.go @@ -1,6 +1,7 @@ package main import ( + "strings" "testing" "time" ) @@ -64,9 +65,9 @@ func TestAWalkFoldedIntoAQueuedBatchIsNotSupersededAndFollowsTheBatchWhenItIsCut } } -func TestADeliverySupersededWhileItsTakerWasABatchFollowsTheTakerOnceItEnds(t *testing.T) { +func TestADeliverySupersededWhileItsTakerWasABatchIsHealedWhenTheTakerEndsDone(t *testing.T) { w, id := folded1786(t) - d := w.delivery(id) + d := w.h.deliveries[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}) @@ -77,8 +78,34 @@ func TestADeliverySupersededWhileItsTakerWasABatchFollowsTheTakerOnceItEnds(t *t 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) + d = w.delivery(id) + // A final state stays final (ADR 0239 §5): the taker's end is kept as DeliveredBy, as issue 442 and 747 do. + if d.State != Superseded || d.DeliveredBy == "" || !memberReached(d) || !d.coveredStop() { + t.Fatalf("superseded by the fault and its taker done, it is %s, delivered by %q, reached %v; wanted superseded, delivered by the taker, reached", + d.State, d.DeliveredBy, memberReached(d)) + } + if st, _ := statusOf(d); st != "success" { + t.Fatalf("its status is %s; wanted success", st) + } +} + +func TestAFinalDeliveryWhoseTakerDoesNotAnswerItIsNeitherHealedNorLoggedEveryPass(t *testing.T) { + w, id := folded1786(t) + d := w.h.deliveries[id] + d.State = Superseded + d.Transitions = append(d.Transitions, Transition{At: w.now, From: Delivering, To: Superseded, Event: EvSuperseded}) + other := aWalk("plan-batch", "novox/other", "ffff", false) + other.State = walkDone + w.h.WalkMoved(w.ctl.put(other)) + w.settleAll() + before := w.logs() + w.later(time30s) + w.settleAll() + if d := w.delivery(id); d.DeliveredBy != "" || d.State != Superseded { + t.Fatalf("healed by a walk that does not answer it: %s %q", d.State, d.DeliveredBy) + } + if n := strings.Count(w.logs(), "does not answer its merge") - strings.Count(before, "does not answer its merge"); n != 0 { + t.Fatalf("logged %d times on a pass over a final record", n) } } diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go index 791d2c26..c2d9b806 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go @@ -351,7 +351,7 @@ func (d *Delivery) coveredStop() bool { switch d.State { case Stopped: return d.Stranded != "" && d.CoveredBy != "" || d.DeliveredBy != "" - case Failed: + case Failed, Superseded: return d.DeliveredBy != "" } return false @@ -397,6 +397,9 @@ func (d *Delivery) strandedEndedSaid() string { // coveredStopSaid is how a covered stop or failure reads: on the forge's status, in its view and note, and in `show`. func (d *Delivery) coveredStopSaid() string { + if d.State == Superseded { + return "delivered after its walk was superseded: " + d.DeliveredBy + } ended := "its stop" if d.State == Failed { ended = "its failure" @@ -1071,3 +1074,13 @@ func mustJSON(v any) json.RawMessage { raw, _ := json.Marshal(v) return raw } + +// 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). +func (d *Delivery) supersededByItsWalk() bool { + if d.State != Superseded || len(d.Transitions) == 0 { + return false + } + t := d.Transitions[len(d.Transitions)-1] + return t.Event == EvSuperseded && (t.From == Published || t.From == Delivering) +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table.go index cd5479e4..b8ed6d08 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table.go @@ -86,8 +86,6 @@ 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) ) @@ -397,23 +395,6 @@ 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)", @@ -752,14 +733,3 @@ 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) -} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table_test.go index 42db6d43..573e4935 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/table_test.go @@ -95,13 +95,6 @@ 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 } @@ -111,13 +104,12 @@ 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) != kept+1 { + if d.State != r.To || got.From != from || got.To != r.To || len(d.Transitions) != 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. @@ -143,7 +135,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, EvFollowed, EvFollowedFailed} + EvHistory, EvRetire, EvCovered, EvLifted, EvRewalk, EvTrunkMoved, EvBackupFailed} for _, from := range append([]State{None}, AllStates...) { for _, ev := range events { for _, act := range []bool{false, true} { @@ -170,8 +162,7 @@ func TestTheTableKeepsItsRules(t *testing.T) { for _, r := range Table { for _, from := range r.From { // Nothing leaves a final state. - // 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 { + if from.Final() { 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.