diff --git a/modules/mesh-delivery/cmd/mesh-delivery/answers.go b/modules/mesh-delivery/cmd/mesh-delivery/answers.go index 3323a5c5..e3c65cb3 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/answers.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/answers.go @@ -298,6 +298,9 @@ func (h *Holder) orphanLines(now time.Time) []StalledLine { Says: fmt.Sprintf("the walk %s of %s at %s waits for this owner's word, and no delivery that has not ended "+ "follows it or is answered by it: `mesh-delivery.show` for the deliveries of %s, and `plans go %s --why …` "+ "starts it by hand", w.ID, w.Repository, shortOf(w.Commit, 8), w.Repository, w.ID)}) + if hold := h.coverHeld(id); hold != "" { + out[len(out)-1].Says += "; it is held back meanwhile: " + hold + } } return out } @@ -388,6 +391,9 @@ func (h *Holder) words() []func() { if stopped, _ := h.carriesStopped(walk); len(stopped) > 0 { h.waitsWhy[walk] = "it carries " + strings.Join(stopped, ", ") + ", whose delivery was stopped: it is " + "asked to leave that merge out first" + if hold := h.coverHeld(walk); hold != "" { + h.waitsWhy[walk] = hold + } h.Logf("[mesh-delivery] the walk %s is not let go: it carries %s, whose delivery was stopped", walk, strings.Join(stopped, ", ")) continue @@ -479,6 +485,10 @@ func (h *Holder) WalksWaiting() []StalledLine { switch { case len(held) > 0: out = append(out, held...) + case h.coverHeld(id) != "": + out = append(out, StalledLine{ID: IDOf(w.Repository, w.Commit), State: Published, Walk: id, For: waited, + Bound: coverAskBound.String(), H2: "none: it is asked to leave the merge out past the bound", + Says: "held back: " + h.coverHeld(id)}) case h.waitsWhy[id] != "": out = append(out, StalledLine{ID: IDOf(w.Repository, w.Commit), State: Published, Walk: id, For: waited, Bound: "0s", H2: "none: it goes at its turn", Says: "not let go yet: " + h.waitsWhy[id]}) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/effects.go b/modules/mesh-delivery/cmd/mesh-delivery/effects.go index e7db65f2..b7a2115f 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/effects.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/effects.go @@ -150,6 +150,9 @@ func statusOf(d *Delivery) (string, string) { case Delivered: return "success", "delivered" case Failed: + if d.coveredStop() { + return "success", clip(d.coveredStopSaid(), 140) // its change reached the machines (novox/hq issue 747) + } return "failure", clip("failed: "+why, 140) case Superseded: return "warning", clip("superseded: "+why, 140) @@ -178,7 +181,7 @@ func ViewBody(d *Delivery, g *Group) string { var b strings.Builder fmt.Fprintf(&b, "%s\n**Delivery** `%s` — **%s** since %s\n\n", viewMarker, d.ID, d.State, d.Since.UTC().Format(time.RFC3339)) if d.coveredStop() { - fmt.Fprintf(&b, "**Covered** after its stop: a later merge that contains it was delivered: %s\n\n", d.CoveredBy) + fmt.Fprintf(&b, "**Covered**: %s\n\n", d.coveredStopSaid()) } if d.HeldWhy != "" && d.State == Held { fmt.Fprintf(&b, "Held for a person: %s — `mesh-delivery.release` with why lets it go on.\n\n", d.HeldWhy) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index da430dc2..383e2d67 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -67,6 +67,13 @@ type Holder struct { // leaveRefused is, by walk, what the controller answered when it refused both to leave a stopped merge out and // to stop the walk whole. leaveRefused map[string]string + // coverAsked are the deliveries that failed or were stopped whose cover by a later finished walk waits for the + // forge's word on containment, worked out each pass (novox/hq issue 747): a walk carrying a merge stopped for one of + // them is held back meanwhile, never asked to leave it out before the answer. + coverAsked map[string]bool + // coverAskedSince is, by delivery, when the forge was first asked about its cover and has not answered: past + // coverAskBound the walk is no longer held back for it, and is asked to leave the merge out (review of issue 747). + coverAskedSince map[string]time.Time // tried counts the owed effects Flush tried since this owner started, by how each ended (novox/hq issue 407). tried owedTries // marks are the open pull requests read with a ready mark, by owner/repo#number (novox/hq issue 722, ready.go). @@ -1112,6 +1119,48 @@ func (h *Holder) plan() []func() { // A published delivery whose walk ended done before its word is read the same way (novox/hq issue 474): what // that walk did not build, a later delivered merge may have; and when the cover test says none did, past the // grace, it is stopped saying so — never left waiting for a start that cannot come. + // A delivery that failed or was stopped is read for a later finished walk of its repository that delivered its + // change after all: a walk that failed in an outage and was resumed, or the walk of a later merge that contains it + // (novox/hq issue 747). Read each tick, from what the walks and the kept deliveries say, so one stuck before this + // was built clears by itself after a restart. + h.coverAsked = map[string]bool{} + if h.coverAskedSince == nil { + h.coverAskedSince = map[string]time.Time{} + } + pending := map[string]bool{} + for _, d := range h.deliveries { + if d.State != Failed && d.State != Stopped || d.coveredStop() { + continue + } + by, ask := h.walkCover(d) + if len(ask) > 0 && by == "" { + pending[d.ID] = true + since, asked := h.coverAskedSince[d.ID] + if !asked { + since = now + h.coverAskedSince[d.ID] = now + } + // Held back only within the bound: a forge that keeps erroring never holds a walk for good. + h.coverAsked[d.ID] = now.Sub(since) < coverAskBound + } + if by != "" { + d.DeliveredBy = by + h.tellCovered(d, now) + continue + } + for _, in := range ask { + if a := h.askContains(d, in, now); a != nil { + asks = append(asks, a) + } + } + } + + for id := range h.coverAskedSince { + if !pending[id] { + delete(h.coverAskedSince, id) + } + } + for _, d := range h.deliveries { ended := d.endedUnstarted() if d.coveredStop() && !d.CoverTold { @@ -1273,7 +1322,7 @@ func (h *Holder) laterOnItsWay(d *Delivery) string { // stays final, and the bus's transition event is not said again. func (h *Holder) tellCovered(d *Delivery, now time.Time) { d.CoverTold = true - line := oneLine(fmt.Sprintf("%s delivery %s: stopped, %s", now.UTC().Format(time.RFC3339), d.ID, d.coveredStopSaid())) + line := oneLine(fmt.Sprintf("%s delivery %s: %s, %s", now.UTC().Format(time.RFC3339), d.ID, d.State, d.coveredStopSaid())) d.Owed = append(d.Owed, Effect{Kind: EffectNote, Commit: d.Commit, Line: line, Since: now}) if d.MergedAs != "" && !sameCommit(d.MergedAs, d.Commit) { d.Owed = append(d.Owed, Effect{Kind: EffectNote, Commit: d.MergedAs, Line: line, Since: now}) @@ -1283,7 +1332,7 @@ func (h *Holder) tellCovered(d *Delivery, now time.Time) { state, desc := statusOf(d) d.owe(Effect{Kind: EffectStatus, Commit: d.Commit, Context: "mesh/delivery", State: state, Line: desc, Since: now}) } - h.Logf("[mesh-delivery] %s: stopped, %s", d.ID, d.coveredStopSaid()) + h.Logf("[mesh-delivery] %s: %s, %s", d.ID, d.State, d.coveredStopSaid()) h.keep(d) } @@ -1294,6 +1343,104 @@ func strandedUncovered(d *Delivery) bool { return d.State == Stopped && d.Stranded != "" && d.CoveredBy == "" } +// walkCover says which later finished walk delivered the change of a delivery that failed or was stopped, or +// nothing, and which walk commits the forge must still be asked about (novox/hq issue 747). A walk covers it when it +// is of its repository and trunk, done, made after its merge, its commit containing its merge — the merge itself, or +// a later one the forge says contains it — and it built every module the delivery's plan moves, each it did not +// build being one the delivery's own walk said unchanged (issue 611). This is the containment "one walk answers every +// merge it contains" uses (issue 362, ADR 0276), read after the fact: a failed walk resumed to done answers its own +// merge, and a later merge's walk answers every merge it contains. A walk that only said a module unchanged for its +// own merges says nothing of this one's, so it covers no module that way. Walks are read from the controller's records +// this owner holds and from every delivery's kept walk, so a walk long ended still counts after a restart. +func (h *Holder) walkCover(d *Delivery) (string, []string) { + if d.State != Failed && d.State != Stopped || d.MergedAs == "" || d.Plan == nil || len(d.Plan.Moved) == 0 { + return "", nil + } + var ask []string + for _, c := range h.doneWalksOf(d.Repository) { + w := c.walk + if !sameBranch(w.Branch, d.Base) || !w.Created.IsZero() && w.Created.Before(d.MergedAt.Add(-time.Minute)) { + continue + } + var missing []string + for _, m := range d.Plan.Moved { + unchanged := false + if d.Walk != nil { + _, unchanged = d.Walk.Unchanged[m] + } + if !containsString(w.Moved, m) && !unchanged { + missing = append(missing, m) + } + } + if len(missing) > 0 { + continue + } + if !sameCommit(c.commit, d.MergedAs) { + yes, known := d.ContainedIn[c.commit] + if !known && !containsString(ask, c.commit) { + ask = append(ask, c.commit) + } + if !yes { + continue + } + } + return fmt.Sprintf("%s (%s at %s into %s, done, contains its merge %s)", w.ID, d.Repository, shortOf(c.commit, 8), + orNone(d.Base), shortOf(d.MergedAs, 8)), nil + } + return "", ask +} + +// doneWalk is a finished walk and the commit of its repository it carried. +type doneWalk struct { + walk *WalkSeen + commit string +} + +// doneWalksOf are the finished walks of a repository this owner knows, oldest first: the controller's records it +// holds, and each delivery's kept walk with the commit of its repository that walk carried for it. +func (h *Holder) doneWalksOf(repository string) []doneWalk { + byID := map[string]doneWalk{} + for id, s := range h.walks { + w := s.walk + if w.Release != nil || w.IsBatch() || w.State != walkDone || !strings.EqualFold(w.Repository, repository) { + continue + } + byID[id] = doneWalk{walk: w.Seen(), commit: w.Commit} + } + for _, o := range h.deliveries { + if o.Walk == nil || o.Walk.State != walkDone || !strings.EqualFold(o.Repository, repository) { + continue + } + if _, known := byID[o.Walk.ID]; known { + continue + } + commit := o.AnsweredBy + if commit == "" && o.MergedAs != "" && sameCommit(o.Walk.Commit, o.MergedAs) { + commit = o.MergedAs + } + if commit == "" { + continue + } + byID[o.Walk.ID] = doneWalk{walk: o.Walk, commit: commit} + } + out := make([]doneWalk, 0, len(byID)) + for _, c := range byID { + out = append(out, c) + } + sort.Slice(out, func(i, j int) bool { + if !out[i].walk.Created.Equal(out[j].walk.Created) { + return out[i].walk.Created.Before(out[j].walk.Created) + } + return out[i].walk.ID < out[j].walk.ID + }) + return out +} + +// stopLapsed is whether a member its group stopped for another is stopped for nothing any more (see stopLapsed). +func (h *Holder) stopLapsed(d *Delivery) bool { + return stopLapsed(d, func(id string) *Delivery { return h.deliveries[id] }) +} + // unwalkedPast is whether a merged delivery has gone without a walk past the bound one opens within, read // again after it: what holds it for a person, unless something already ends it. func (h *Holder) unwalkedPast(d *Delivery, now time.Time) bool { @@ -1736,7 +1883,8 @@ func (h *Holder) turnsOf(g *Group, members []*Delivery, waits map[string]carryWa // A superseded member a later delivered walk carried counts as reached, as in GroupState: a group counts a // covered member as delivered, so the member after it gets its turn (ADR 0270 decision 2, novox/hq // issues 442 and 445). - if d.State.Reached() || d.State == Superseded && d.CoveredBy != "" { + // A stop or failure a later walk delivered since counts as reached too (novox/hq issues 480 and 747). + if memberReached(d) { continue } if sharing != "" && (d.Walk == nil || d.Walk.ID != sharing) { @@ -1761,7 +1909,7 @@ func (h *Holder) turnsOf(g *Group, members []*Delivery, waits map[string]carryWa } if w.why != "" { for _, later := range ordered[i+1:] { - if a := h.stopInGroup(byID[later.ID], fmt.Sprintf("its group %s stopped: %s superseded, and %s", g.ID, + if a := h.stopInGroup(byID[later.ID], d.ID, fmt.Sprintf("its group %s stopped: %s superseded, and %s", g.ID, d.ID, w.why)); a != nil { asks = append(asks, a) } @@ -1788,9 +1936,13 @@ func (h *Holder) turnsOf(g *Group, members []*Delivery, waits map[string]carryWa continue } return asks + case h.stopLapsed(d): + // Stopped for a member whose change reached the machines since: it stops nothing more. Its own change is + // delivered by a later walk of its repository that contains it, which then covers it (novox/hq issue 747). + continue case d.State == Failed || d.State == Stopped: for _, later := range ordered[i+1:] { - if a := h.stopInGroup(byID[later.ID], fmt.Sprintf("its group %s stopped: %s %s", g.ID, d.ID, d.State)); a != nil { + if a := h.stopInGroup(byID[later.ID], d.ID, fmt.Sprintf("its group %s stopped: %s %s", g.ID, d.ID, d.State)); a != nil { asks = append(asks, a) } } @@ -2028,7 +2180,7 @@ func mayCarry(c, d *Delivery, module string) bool { // 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() { +func (h *Holder) stopInGroup(d *Delivery, cause, why string) func() { if d == nil || d.State.Final() { return nil } @@ -2037,6 +2189,7 @@ func (h *Holder) stopInGroup(d *Delivery, why string) func() { h.refuse(err) return nil } + d.StoppedFor = cause // the member it was stopped for: once that one is delivered, the stop lapses (novox/hq issue 747) h.moved(d, []Transition{t}) // 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. @@ -2118,6 +2271,12 @@ func (h *Holder) carriesStopped(walk string) ([]string, string) { if last := d.Transitions[len(d.Transitions)-1]; last.Event != EvStop || last.By == "" { continue } + // A stop whose change a later walk delivered since, or one its group made for a member delivered since, + // holds nothing back: the group stops a walk only for a member whose change is still undelivered (novox/hq + // issue 747). + if d.coveredStop() || h.stopLapsed(d) { + 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) @@ -2135,6 +2294,51 @@ func (h *Holder) carriesStopped(walk string) ([]string, string) { return out, why } +// coverAwaited is whether a stopped merge a walk carries, given as `@`, waits for the forge's +// word on whether a later finished walk delivered it or the member its group stopped it for (novox/hq issue 747). +// It says since when the forge was first asked, the earliest of them. Held only within coverAskBound of the first +// ask: past it the forge's silence or error lets the walk be asked to leave the merge out. Under the lock. +func (h *Holder) coverAwaited(merges []string) (time.Time, bool) { + var since time.Time + awaited := false + for _, d := range h.deliveries { + if d.State != Stopped || d.MergedAs == "" || !containsString(merges, d.Repository+"@"+d.MergedAs) { + continue + } + for _, id := range []string{d.ID, d.stoppedFor()} { + if id == "" || !h.coverAsked[id] { + continue + } + if at := h.coverAskedSince[id]; !awaited || at.Before(since) { + since = at + } + awaited = true + } + } + return since, awaited +} + +// coverHeld says why a walk is held back while the forge is asked whether a later finished walk delivered a stopped +// merge it carries (novox/hq issue 747), or nothing when it is not. Said by `waits`, `stalled` and `stalled walks`, so +// a forge that cannot answer is loud. Under the lock. +func (h *Holder) coverHeld(walk string) string { + without, _ := h.carriesStopped(walk) + if len(without) == 0 { + return "" + } + since, awaited := h.coverAwaited(without) + if !awaited { + return "" + } + return fmt.Sprintf("it carries %s, whose delivery was stopped, and the forge is asked since %s whether a later "+ + "finished walk delivered it; it is asked to leave the merge out if no answer comes by %s (novox/hq issue 747)", + strings.Join(without, ", "), since.Local().Format("15:04:05"), since.Add(coverAskBound).Local().Format("15:04:05")) +} + +// coverAskBound is how long a walk carrying a stopped merge is held back while the forge is asked whether a later +// finished walk delivered it (novox/hq issue 747); past it, the walk is asked to leave the merge out. +const coverAskBound = 10 * time.Minute + // lastWhyOf is the reason of a delivery's last transition. func lastWhyOf(d *Delivery) string { if len(d.Transitions) == 0 { @@ -2170,6 +2374,9 @@ func (h *Holder) leaveStopped() []func() { if without, _ := h.carriesStopped(id); len(without) > 0 { said := fmt.Sprintf("its walk %s is not let go: it carries %s, whose delivery was stopped, and is asked to "+ "leave it out (novox/hq ADR 0299)", id, strings.Join(without, ", ")) + if hold := h.coverHeld(id); hold != "" { + said = "its walk " + id + " is held back: " + hold + } if r := h.leaveRefused[id]; r != "" { said += "; the controller refused: " + r } @@ -2193,6 +2400,9 @@ func (h *Holder) leaveStopped() []func() { delete(h.leaveRefused, id) continue } + if _, awaited := h.coverAwaited(without); awaited { + continue // held back, not stopped, until the forge says whether a later walk delivered it (issue 747) + } // 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 { @@ -2314,18 +2524,28 @@ func againWords(a StopAnswer) string { func GroupState(g *Group, members []*Delivery) (string, string) { count := map[State]int{} reached, ended := 0, 0 - var uncovered []string + var uncovered, lapsed []string + byID := map[string]*Delivery{} + for _, d := range members { + byID[d.ID] = d + } for _, d := range members { - count[d.State]++ if d.State.Final() { ended++ } switch { - case d.State.Reached(), d.State == Superseded && d.CoveredBy != "": + case memberReached(d): + // Reached, whatever its final state: a failure or stop a later walk delivered since is no failure of the + // group (novox/hq issue 747). reached++ + continue + case stopLapsed(d, func(id string) *Delivery { return byID[id] }): + lapsed = append(lapsed, d.ID) + continue case d.State == Superseded: uncovered = append(uncovered, d.ID) } + count[d.State]++ } n := len(members) switch { @@ -2335,6 +2555,11 @@ func GroupState(g *Group, members []*Delivery) (string, string) { return "stopped", "a member was stopped" case n > 0 && reached == n: return "delivered", "every member is delivered" + case g.Closed && len(lapsed) > 0 && reached+len(lapsed) == n: + // Stopped for a member delivered since: its own change waits for a later walk of its repository, which then + // covers it (novox/hq issue 747). + return "delivering", fmt.Sprintf("%d of %d delivered; %s, stopped for a member delivered since, waits for a "+ + "later walk of its repository", reached, n, strings.Join(lapsed, ", ")) case g.Closed && n > 0 && ended == n: return "stopped", supersededWhy + strings.Join(uncovered, ", ") case g.Closed && g.Check != nil && (g.Check.OutOfDate != "" || g.Check.Verdict == "" || diff --git a/modules/mesh-delivery/cmd/mesh-delivery/issue747_test.go b/modules/mesh-delivery/cmd/mesh-delivery/issue747_test.go new file mode 100644 index 00000000..7646ec14 --- /dev/null +++ b/modules/mesh-delivery/cmd/mesh-delivery/issue747_test.go @@ -0,0 +1,258 @@ +package main + +import ( + "strings" + "testing" + "time" +) + +// novox/hq issue 747: in the group fix/a-module-moves-only-when-its-program-changes, the controller's member #336 +// (merged as 5d22d7df) failed at 22:32 during an outage of the forge's tools. The walk of the controller's next merge, +// #339 at 60cc9ec3, which contains 5d22d7df, failed with it and was resumed to done by a rebuild. mesh-delivery still +// held #336 failed, so its group stopped every later walk of the catalogue that carried the catalogue member's merge +// 2edef5a0: the catalogue could deliver nothing. A failed or stopped delivery is covered once a later finished walk of +// its repository that contains its merge built its modules, and a group stops a walk only for a member whose change is +// still undelivered. +// +// The shape is the live state of 2026-10-14 23:07, as mesh-delivery.show gave it, read back after a restart: the +// controller holds none of the walks of the night any more, only the walk of the catalogue's next merge. + +const ( + g747 = "fix/a-module-moves-only-when-its-program-changes" + ctl747, cat747 = "novox/mesh-controller", "novox/mesh-catalog" + head336, merge336 = "b34055f580d4e2a4", "5d22d7df86c73452" + head339, merge339 = "e4acab32d6a20ade", "60cc9ec363dfa7c6" + head301, merge301 = "d94764f00168471f", "2edef5a00ae05605" + merge307 = "0f76b6e2aaaaaaaa" +) + +var ( + at747 = time.Date(2026, 10, 14, 20, 21, 6, 0, time.UTC) + id336, id339, id301 = IDOf(ctl747, head336), IDOf(ctl747, head339), IDOf(cat747, head301) + unchangedRouteProxy747 = map[string]string{"route-proxy": "its build source holds none of the changed files"} +) + +// night747 is the stuck state, with the forge saying, or not, that 60cc9ec3 contains 5d22d7df. +func night747(t *testing.T, contained bool) *world { return night747With(t, contained, nil) } + +// night747With is night747 with #339's walk having built only the modules given, saying the others unchanged. +func night747With(t *testing.T, contained bool, built []string) *world { + t.Helper() + return night747Over(t, func(f *fakeForge) { f.contains[merge336+" in "+merge339] = contained }, built) +} + +// night747Over is night747With with the forge set as given: one that is not told the pair cannot say. +func night747Over(t *testing.T, set func(*fakeForge), built []string) *world { + t.Helper() + at := at747 + failedAt := at.Add(11 * time.Minute) + store := newMemStore() + moved339, unchanged339 := []string{"build-agent", "mesh-controller"}, unchangedRouteProxy747 + if built != nil { + moved339, unchanged339 = built, map[string]string{} + for _, m := range []string{"build-agent", "mesh-controller", "route-proxy"} { + if !containsString(built, m) { + unchanged339[m] = "its build source holds none of the files #339 changed" + } + } + } + m336 := &Delivery{ID: id336, Repository: ctl747, Commit: head336, Number: 336, Base: "main", Branch: g747, Group: g747, + State: Failed, Since: failedAt, Created: at.Add(-time.Hour), MergedAs: merge336, MergedAt: at, + AnsweredBy: merge336, Check: &Verdict{Gate: "pass"}, + Plan: planMoving(ctl747, "build-agent", "mesh-controller", "route-proxy"), + Walk: &WalkSeen{ID: "plan-336", Commit: merge336, State: walkFailed, Branch: "main", MovedSaid: true, + Note: "mesh-controller failed to build in tier 0", Created: at.Add(7 * time.Minute), Tiers: 2, + Unchanged: unchangedRouteProxy747}, + Transitions: []Transition{{At: failedAt, From: Delivering, To: Failed, Event: EvFailed, + Why: "its walk failed, carrying its own commit"}}} + m339 := &Delivery{ID: id339, Repository: ctl747, Commit: head339, Number: 339, Base: "main", Branch: "fix/726", + State: Failed, Since: failedAt, Created: at.Add(-10 * time.Minute), MergedAs: merge339, MergedAt: at.Add(7 * time.Minute), + AnsweredBy: merge339, Check: &Verdict{Gate: "pass"}, Plan: planMoving(ctl747, "mesh-controller"), + // Its walk failed with #336's, and a rebuild resumed it to done: the record it keeps says so. + Walk: &WalkSeen{ID: "plan-339", Commit: merge339, State: walkDone, Branch: "main", MovedSaid: true, + Moved: moved339, Unchanged: unchanged339, + Created: at.Add(11 * time.Minute), Tiers: 2, Tier: 2}, + Transitions: []Transition{{At: failedAt, From: Delivering, To: Failed, Event: EvFailed, + Why: "its walk failed, carrying its own commit"}}} + m301 := &Delivery{ID: id301, Repository: cat747, Commit: head301, Number: 301, Base: "main", Branch: g747, Group: g747, + State: Stopped, Since: failedAt, Created: at.Add(-2 * time.Hour), MergedAs: merge301, MergedAt: at.Add(7 * time.Second), + Check: &Verdict{Gate: "pass"}, Plan: planMoving(cat747, "build-agent", "route-proxy"), + Transitions: []Transition{{At: failedAt, From: Ready, To: Stopped, Event: EvStop, By: byOwner, + Why: "its group " + g747 + " stopped: " + id336 + " failed"}}} + for _, d := range []*Delivery{m336, m339, m301} { + if err := store.PutDelivery(d); err != nil { + t.Fatal(err) + } + } + g := &Group{ID: g747, Members: []string{id301, id336}, Closed: true, Order: []string{id336, id301}, Said: "failed", + OrderOf: commitsOf([]*Delivery{m301, m336}), Check: &GroupCheck{Asked: "check-1", At: at.Add(-time.Minute), + Verdict: "pass", Summary: "both compose", Commits: commitsOf([]*Delivery{m301, m336})}} + if err := store.PutGroup(g); err != nil { + t.Fatal(err) + } + ctlr := newFakeController() + // The catalogue's next merge: its walk carries 2edef5a0, merged before it and never walked, and waits for its word. + ctlr.put(Walk{ID: "plan-cat", Repository: cat747, Branch: "main", Commit: merge307, Created: at.Add(46 * time.Minute), + State: walkBuilding, Tiers: [][]string{{"gitea"}}, Modules: map[string]*WalkModule{"gitea": {}}, + Delivery: &WalkDelivery{Awaits: "mesh-delivery", Merges: []WalkMerge{ + {Repository: cat747, Commit: merge301, Carried: merge307}, {Repository: cat747, Commit: merge307}}}}) + ctlr.mergedAt[merge301], ctlr.mergedAt[merge307] = m301.MergedAt, at.Add(46*time.Minute) + forge := newFakeForge() + set(forge) + w := newWorldOver(t, store, ctlr, forge, at.Add(47*time.Minute)) + if err := w.h.Reconcile(); err != nil { + t.Fatal(err) + } + w.settleAll() + return w +} + +func groupState747(w *world) (string, string) { + g := w.h.groups[g747] + return GroupState(g, w.h.membersOf(g)) +} + +// **A failed member is covered by a later finished walk that contains it**: #339's walk, done at 60cc9ec3, which the +// forge says contains 5d22d7df, built build-agent and mesh-controller, and #336's own walk said route-proxy unchanged. +// #336 stays failed, reads covered by that walk on its pull request, and its group no longer reads failed. #339, whose +// own walk was resumed to done, is covered by it too. +func TestAFailedMemberIsCoveredByALaterDoneWalkThatContainsIt(t *testing.T) { + w := night747(t, true) + d := w.delivery(id336) + if state, desc := statusOf(d); d.State != Failed || state != "success" || !strings.Contains(desc, "covered after its failure") || + !strings.Contains(desc, "plan-339") { + t.Fatalf("#336's status reads %s %q", state, desc) + } + if s := statusOn(w, ctl747, head336); !strings.HasPrefix(s, "success") { + t.Fatalf("the forge was told %q of #336", s) + } + if state, desc := statusOf(w.delivery(id339)); state != "success" || !strings.Contains(desc, "plan-339") { + t.Fatalf("#339 reads %s %q; want it covered by its own walk, resumed to done", state, desc) + } + if state, why := groupState747(w); state == "failed" || state == "stopped" { + t.Fatalf("the group reads %s: %s", state, why) + } + // Told once, and kept: a restart tells nothing again. + notes := len(w.forge.notesOn(merge336)) + again := newWorldOver(t, w.store, w.ctl, w.forge, w.now.Add(time.Minute)) + again.settleAll() + if state, _ := statusOf(again.delivery(id336)); len(w.forge.notesOn(merge336)) != notes || state != "success" { + t.Fatalf("a restart told it again or forgot it: %d notes, was %d", len(w.forge.notesOn(merge336)), notes) + } +} + +// **The group then no longer stops the other repository's walk**: the catalogue member was stopped for #336 alone, +// so the walk of the catalogue's next merge, which carries its merge, is neither stopped nor held back, and the +// group waits for that walk to deliver the catalogue member's change. +func TestACoveredMemberNoLongerStopsTheOtherRepositorysWalk(t *testing.T) { + w := night747(t, true) + if len(w.ctl.withouts) > 0 || containsString(w.ctl.stops, "plan-cat") { + t.Fatalf("the catalogue's walk was stopped: without %v, stops %v", w.ctl.withouts, w.ctl.stops) + } + if stopped, _ := w.h.carriesStopped("plan-cat"); len(stopped) > 0 { + t.Fatalf("the catalogue's walk is held back for %v", stopped) + } + if state, why := groupState747(w); state != "delivering" || !strings.Contains(why, id301) { + t.Fatalf("the group reads %s: %s; want delivering, the catalogue member waiting for a later walk", state, why) + } + // Once a later walk of the catalogue that contains its merge built its modules, it is covered, and the group is + // delivered. + w.forge.contains[merge301+" in "+merge307] = true + r := w.ctl.walk("plan-cat") + sent := w.now + r.Modules = map[string]*WalkModule{} + for _, m := range []string{"build-agent", "route-proxy", "gitea"} { + r.Modules[m] = &WalkModule{State: "built", SentAt: &sent} + } + r.State = walkDone + w.h.WalkMoved(w.ctl.put(r)) + w.later(time.Minute) + w.settleAll() + if state, desc := statusOf(w.delivery(id301)); state != "success" || !strings.Contains(desc, "plan-cat") { + t.Fatalf("the catalogue member reads %s %q", state, desc) + } + if state, why := groupState747(w); state != "delivered" { + t.Fatalf("the group reads %s: %s", state, why) + } +} + +// **A failed member not contained still stops it**: the forge says 60cc9ec3 does not contain 5d22d7df, so #336's change +// never reached the machines; it stays failed and uncovered, its group failed, and the catalogue's walk that carries +// the stopped member's merge is asked to leave it out. +func TestAFailedMemberNotContainedStillStopsTheOtherRepositorysWalk(t *testing.T) { + w := night747(t, false) + d := w.delivery(id336) + if state, desc := statusOf(d); d.State != Failed || state != "failure" { + t.Fatalf("#336 is %s and reads %s %q", d.State, state, desc) + } + if state, _ := groupState747(w); state != "failed" { + t.Fatalf("the group reads %s", state) + } + if len(w.ctl.withouts) == 0 || !strings.HasPrefix(w.ctl.withouts[0], "plan-cat without "+cat747+"@"+merge301) { + t.Fatalf("the catalogue's walk was not asked to leave the stopped merge out: %v", w.ctl.withouts) + } +} + +// **A later walk that only said a module unchanged covers nothing of an earlier merge that moves it**: #339's walk +// built mesh-controller alone and said build-agent unchanged for #339's own files, which says nothing of #336's; only +// #336's own walk says that of its change (route-proxy). #336 stays failed and its group still stops the catalogue. +func TestALaterWalksUnchangedCoversNoModuleOfAnEarlierMerge(t *testing.T) { + w := night747With(t, true, []string{"mesh-controller"}) + if state, desc := statusOf(w.delivery(id336)); state != "failure" { + t.Fatalf("#336 reads %s %q, though no walk built build-agent for it", state, desc) + } + if state, _ := groupState747(w); state != "failed" { + t.Fatalf("the group reads %s", state) + } + if len(w.ctl.withouts) == 0 { + t.Fatal("the catalogue's walk was not asked to leave the stopped merge out") + } +} + +// **A forge that cannot say holds the walk only within the bound** (review of issue 747): while the forge errors on +// whether 60cc9ec3 contains 5d22d7df, the catalogue's walk is held back, loudly, neither stopped nor asked to leave the +// merge out, and #336 still reads failed. Past the bound the forge's silence holds nothing back any more: the walk +// is asked to leave the stopped merge out, as with no later walk at all. +func TestAForgeThatCannotSayHoldsTheWalkOnlyWithinTheBound(t *testing.T) { + for _, c := range []struct { + name string + set func(*fakeForge) + }{ + {"erroring", func(f *fakeForge) { f.containsDown = true }}, + {"not answering for the pair", func(*fakeForge) {}}, + } { + t.Run(c.name, func(t *testing.T) { + w := night747Over(t, c.set, nil) + if len(w.ctl.withouts) > 0 || len(w.ctl.stops) > 0 { + t.Fatalf("within the bound the walk was not held: without %v, stops %v", w.ctl.withouts, w.ctl.stops) + } + if state, desc := statusOf(w.delivery(id336)); state != "failure" { + t.Fatalf("#336 reads %s %q while the forge cannot say", state, desc) + } + said := "" + for _, l := range w.h.WalksWaiting() { + if l.Walk == "plan-cat" { + said += l.Says + } + } + w.later(3 * time.Minute) // past the grace an unheld walk is given, so `stalled` lists it + w.settleAll() + for _, l := range w.h.Stalled() { + if l.Walk == "plan-cat" { + said += " | " + l.Says + } + } + if strings.Count(said, "the forge is asked since") != 2 { + t.Fatalf("the hold is not said by `stalled walks` and `stalled`: %q", said) + } + if len(w.ctl.withouts) > 0 || len(w.ctl.stops) > 0 { + t.Fatalf("within the bound the walk was not held: without %v, stops %v", w.ctl.withouts, w.ctl.stops) + } + w.later(coverAskBound + time.Minute) + w.settleAll() + if len(w.ctl.withouts) == 0 || !strings.HasPrefix(w.ctl.withouts[0], "plan-cat without "+cat747+"@"+merge301) { + t.Fatalf("past the bound the walk was not asked to leave the stopped merge out: %v", w.ctl.withouts) + } + }) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/model.go b/modules/mesh-delivery/cmd/mesh-delivery/model.go index ec3e2e1d..2d2a0c67 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/model.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/model.go @@ -83,6 +83,14 @@ type Delivery struct { // CoverTold is whether the forge was told that such a stop was covered since: its status, its view and its note // (novox/hq issue 480). Kept, so a restart tells it once, and a stop covered before this was built is told too. CoverTold bool `json:"cover_told,omitempty"` + // DeliveredBy says, for a delivery that failed or was stopped, the later finished walk of its repository whose + // commit contains its merge and which built every module it moves — each it did not build, its own walk said + // unchanged: its change reached the machines after all (novox/hq issue 747). Its state stays final, as a covered + // stop's does (issue 480), and it reads covered: its group counts it as delivered. + DeliveredBy string `json:"delivered_by,omitempty"` + // StoppedFor is, for a member its group stopped, the member whose end stopped it (novox/hq issue 747): once that + // member's change reached the machines, the stop has lapsed, and holds no walk of its repository back. + StoppedFor string `json:"stopped_for,omitempty"` // TurnAwaits says, for a group member whose turn waits for a later merge to carry a superseded member before it, // which merges and until when; TurnAwaitsUntil is that moment, which bounds the wait (ADR 0276 decision 9, // novox/hq issue 445). `show` shows both, and `stalled` says them past that moment. @@ -303,18 +311,77 @@ func (d *Delivery) FromForgesWord() bool { // shownOnItsPullRequest is whether its view and status are kept on its pull request: not for history, whose // pull request the forge may no longer hold — or holds under its number another one. -// coveredStop is whether a delivery stopped because its walk ended done before its word without building its modules -// (novox/hq issue 474) was covered since: a later merge into its trunk that contains it was delivered, carrying every -// module it moves to every machine running it (ADR 0270's cover test, kept as CoveredBy). Its state stays stopped — -// final, as a covered superseded member stays superseded (issue 442) — and it reads covered, by that merge (novox/hq -// issue 480). A stop for any other reason never reads covered. +// coveredStop is whether a delivery that ended stopped or failed was covered since, and reads covered. A stop +// because its walk ended done before its word without building its modules (novox/hq issue 474) is covered when a +// later merge into its trunk that contains it was delivered, carrying every module it moves to every machine running +// it (ADR 0270's cover test, kept as CoveredBy; issue 480). Any stop or failure is covered when a later finished walk +// of its repository whose commit contains its merge built its modules (DeliveredBy, novox/hq issue 747). Its state +// stays final, as a covered superseded member stays superseded (issue 442), and it reads covered, by that merge or +// walk. A person's stop with a cover found only while it was held keeps its error. func (d *Delivery) coveredStop() bool { - return d.State == Stopped && d.Stranded != "" && d.CoveredBy != "" + switch d.State { + case Stopped: + return d.Stranded != "" && d.CoveredBy != "" || d.DeliveredBy != "" + case Failed: + return d.DeliveredBy != "" + } + return false } -// coveredStopSaid is how a covered stop reads: on the forge's status, in its view and note, and in `show`. +// 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 { - return "covered after its stop: a later merge that contains it was delivered: " + d.CoveredBy + ended := "its stop" + if d.State == Failed { + ended = "its failure" + } + if d.Stranded != "" && d.CoveredBy != "" { + return "covered after " + ended + ": a later merge that contains it was delivered: " + d.CoveredBy + } + return "covered after " + ended + ": a later walk that contains it was delivered: " + d.DeliveredBy +} + +// memberReached is whether a group counts a member as delivered: its change reached the machines, or had nothing to +// reach them with — delivered, history or covered, a superseded member a later delivered walk carried (issue 442), +// or a stop or failure covered since (issues 480 and 747). +func memberReached(d *Delivery) bool { + return d.State.Reached() || d.State == Superseded && d.CoveredBy != "" || d.coveredStop() +} + +// stoppedFor is the member whose end stopped this one in its group: kept since novox/hq issue 747, and read from the +// stop's reason for a stop made before ("its group stopped: …"). +func (d *Delivery) stoppedFor() string { + if d.StoppedFor != "" || d.State != Stopped || d.Group == "" { + return d.StoppedFor + } + prefix := "its group " + d.Group + " stopped: " + for i := len(d.Transitions) - 1; i >= 0; i-- { + t := d.Transitions[i] + if t.Event != EvStop { + continue + } + if t.By != byOwner || !strings.HasPrefix(t.Why, prefix) { + return "" + } + member, _, _ := strings.Cut(strings.TrimPrefix(t.Why, prefix), " ") + return strings.TrimRight(member, ",;:") + } + return "" +} + +// stopLapsed is whether a member its group stopped for another member is stopped for nothing any more: that member's +// change reached the machines since — delivered, or its failure covered by a later walk (novox/hq issue 747). Its own +// change is still to deliver, by a later walk of its repository that contains it; its stop holds none back. find +// gives a delivery by its id. +func stopLapsed(d *Delivery, find func(string) *Delivery) bool { + if d.State != Stopped || d.coveredStop() { + return false + } + by := d.stoppedFor() + if by == "" || by == d.ID { + return false + } + o := find(by) + return o != nil && memberReached(o) } func (d *Delivery) shownOnItsPullRequest() bool { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go index b95aacd2..b2dfed42 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go @@ -72,6 +72,10 @@ func (h *Holder) waitsFor(d *Delivery) string { return "its walk: " + clip(d.Walk.Note, 120) } case Stopped: + if h.stopLapsed(d) { + return "a later walk of its repository that contains its merge: its group stopped it for " + d.stoppedFor() + + ", whose change was delivered since (novox/hq issue 747)" + } if strandedUncovered(d) { return "a later delivered merge to carry " + strings.Join(d.notMovedBy(), ", ") + ": it is stopped, and " + "listed by stalled until one does (novox/hq issue 480)"