mesh-delivery: a member after a superseded one waits while a later merge can carry it
mesh/merge-gate pass: builds mesh-delivery → novox; no bus step; every machine composes with the change as it did without (4 of 4 compose)
mesh/repo-check pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-check-typescript.sh (typescript)
mesh/delivery superseded: a newer head of the same pull request
mesh/merge-gate pass: builds mesh-delivery → novox; no bus step; every machine composes with the change as it did without (4 of 4 compose)
mesh/repo-check pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-check-typescript.sh (typescript)
mesh/delivery superseded: a newer head of the same pull request
Superseded is no failure: the later merge that took its walk over may still deliver its change, and a covered member counts as delivered (ADR 0270). So the members after it wait while such a merge is open within its own bound, take their turn once its delivered walk covers the member, and are stopped, saying why, only when none can carry it (novox/hq issue 445).
This commit is contained in:
@@ -1393,7 +1393,9 @@ func (h *Holder) inOrder(g *Group, members []*Delivery) []Member {
|
||||
// groupTurns walks a delivering group in its order: the next member is let go only when every member before
|
||||
// it is delivered; a member that failed or was stopped stops every member after it, naming it. What was
|
||||
// delivered before it stays, each judged at its own gate (ADR 0239 decision 4); the failed member's builds
|
||||
// were put back at its gate by the controller.
|
||||
// were put back at its gate by the controller. A superseded member counts as delivered once a later delivered
|
||||
// walk carried it, holds the members after it while a later merge can still carry it, and stops them, saying
|
||||
// why, only when none can (novox/hq issue 445).
|
||||
func (h *Holder) groupTurns(g *Group, members []*Delivery) []func() {
|
||||
ordered := h.inOrder(g, members)
|
||||
byID := map[string]*Delivery{}
|
||||
@@ -1409,18 +1411,33 @@ func (h *Holder) groupTurns(g *Group, members []*Delivery) []func() {
|
||||
sharing := ""
|
||||
for i, m := range ordered {
|
||||
d := byID[m.ID]
|
||||
if d.State.Reached() {
|
||||
// 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 != "" {
|
||||
continue
|
||||
}
|
||||
if sharing != "" && (d.Walk == nil || d.Walk.ID != sharing) {
|
||||
return asks
|
||||
}
|
||||
switch {
|
||||
case d.State == Failed || d.State == Stopped || d.State == Superseded:
|
||||
// A superseded member is an end here, and it stops the members after it even when a later delivered
|
||||
// walk carried it. GroupState counts such a member as reached (novox/hq issue 442), so for a carried
|
||||
// member the two do not agree: the members after it are stopped all the same, and the group then reads
|
||||
// stopped for them. That gap is novox/hq issue 445, not closed here.
|
||||
case d.State == Superseded:
|
||||
// Superseded is no failure: the merge that took its walk over may still deliver its change. While a later
|
||||
// merge into its trunk that can carry it is open, within that merge's own bound, the members after it
|
||||
// wait, as for a member still delivering (ADR 0239 decision 4: the next starts when the one before is
|
||||
// delivered). Only when none can carry it are they stopped, saying why (novox/hq issue 445).
|
||||
awaited, why := h.carryAwaited(d, h.Now())
|
||||
if awaited != "" {
|
||||
return asks
|
||||
}
|
||||
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, d.ID,
|
||||
why)); a != nil {
|
||||
asks = append(asks, a)
|
||||
}
|
||||
}
|
||||
return asks
|
||||
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 {
|
||||
asks = append(asks, a)
|
||||
@@ -1468,6 +1485,74 @@ func (h *Holder) groupTurns(g *Group, members []*Delivery) []func() {
|
||||
return asks
|
||||
}
|
||||
|
||||
// containsGrace is how long a later merge whose walk was delivered may still be awaited as a superseded member's
|
||||
// carrier while the forge has not said whether its merge commit contains the member's: as long as a published
|
||||
// delivery may wait for its turn (Bounds[Published]). A forge that cannot answer means "not yet covered" (ADR
|
||||
// 0270), and without this bound the members after it would wait on that answer for ever (novox/hq issue 445).
|
||||
const containsGrace = 30 * time.Minute
|
||||
|
||||
// carryAwaited says which later merges into a superseded member's trunk may still carry every module it moves —
|
||||
// the cover GroupState counts as reached (ADR 0270's covered test, novox/hq issue 442) — or, when none can, why.
|
||||
// A later merge of the same repository into the same trunk on a passing check, whose plan moves the module and
|
||||
// whose merge commit the forge has not said leaves this one out, can carry it: while it is merged and not yet
|
||||
// ended, not held for a person, and within its own state's bound (ADR 0239 decision 9, ADR 0276 decision 9: every
|
||||
// wait has a bound); once delivered, while the forge has not yet said whether it contains this merge, within
|
||||
// containsGrace. So the wait is never longer than the carrier's own bound; a carrier superseded in turn hands the
|
||||
// wait to the later merge that took it over, which is a carrier of its own (novox/hq issue 445).
|
||||
func (h *Holder) carryAwaited(d *Delivery, now time.Time) (string, string) {
|
||||
if d.MergedAs == "" || d.MergedAt.IsZero() || d.Plan == nil || len(d.Plan.Moved) == 0 {
|
||||
return "", "nothing records which modules it moves, so no later merge can carry it"
|
||||
}
|
||||
var awaited []string
|
||||
for _, module := range d.Plan.Moved {
|
||||
found := false
|
||||
var not []string
|
||||
for _, c := range h.deliveries {
|
||||
if !mayCarry(c, d, module) {
|
||||
continue
|
||||
}
|
||||
contained, known := d.ContainedIn[c.MergedAs]
|
||||
switch {
|
||||
case known && !contained:
|
||||
not = append(not, c.ID+" does not contain its merge")
|
||||
case c.State == Delivered && carries(c, d, module) && known:
|
||||
found = true // covered for this module; another module is what is still awaited
|
||||
case c.State == Delivered && carries(c, d, module) && now.Sub(c.Since) <= containsGrace:
|
||||
found = true
|
||||
awaited = append(awaited, c.ID+" (delivered; whether it contains its merge is asked of the forge)")
|
||||
case c.State == Delivered && carries(c, d, module):
|
||||
not = append(not, fmt.Sprintf("the forge did not say within %s whether %s contains its merge", containsGrace, c.ID))
|
||||
case c.State.Final() || c.State == Held:
|
||||
// ended without carrying it, or held for a person: no carrier to wait for
|
||||
case pastBound(c, now):
|
||||
b, _ := boundOf(c)
|
||||
not = append(not, fmt.Sprintf("%s is past its bound (%s %s)", c.ID, c.State, b.For))
|
||||
default:
|
||||
found = true
|
||||
awaited = append(awaited, fmt.Sprintf("%s (%s)", c.ID, c.State))
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
why := fmt.Sprintf("no later merge into %s can carry %s", d.Base, module)
|
||||
if len(not) > 0 {
|
||||
sort.Strings(not)
|
||||
why += ": " + strings.Join(not, "; ")
|
||||
}
|
||||
return "", why
|
||||
}
|
||||
}
|
||||
sort.Strings(awaited)
|
||||
return strings.Join(slices.Compact(awaited), ", "), ""
|
||||
}
|
||||
|
||||
// mayCarry is whether c is a later merge that could carry module for d, by what is known before its walk ends: of
|
||||
// the same repository, merged later into the same trunk, on its own passing check, its plan moving the module.
|
||||
func mayCarry(c, d *Delivery, module string) bool {
|
||||
return c.ID != d.ID && strings.EqualFold(c.Repository, d.Repository) && strings.EqualFold(c.Base, d.Base) &&
|
||||
c.MergedAs != "" && !sameCommit(c.MergedAs, d.MergedAs) && c.MergedAt.After(d.MergedAt) && c.Check.Passes() &&
|
||||
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.
|
||||
func (h *Holder) stopInGroup(d *Delivery, why string) func() {
|
||||
if d == nil || d.State.Final() {
|
||||
@@ -1499,8 +1584,8 @@ func (h *Holder) stopInGroup(d *Delivery, why string) func() {
|
||||
// 0270's covered test); a closed group whose members all ended is then delivered, and otherwise stopped, naming
|
||||
// the member. A group with nothing in progress never reads delivering.
|
||||
//
|
||||
// groupTurns does not yet agree for a carried member: it stops the members after any superseded one, carried or
|
||||
// not, and a stopped member makes the group read stopped here. That gap is novox/hq issue 445.
|
||||
// groupTurns agrees: the members after a carried member take their turn, and they wait while a later merge can still
|
||||
// carry it (novox/hq issue 445).
|
||||
func GroupState(g *Group, members []*Delivery) (string, string) {
|
||||
count := map[State]int{}
|
||||
reached, ended := 0, 0
|
||||
|
||||
@@ -0,0 +1,161 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// novox/hq issue 445: in a delivery group ordered [A, B], A's walk was superseded by the walk of a later merge C
|
||||
// of A's repository that does not answer A's merge. groupTurns stopped B at once, although C's walk could still
|
||||
// carry A's modules and A then counts as reached (ADR 0270 decision 2, issue 442). While a later walk that can
|
||||
// carry A is open, B waits; once that walk is delivered and covers A, B takes its turn; when no later merge can
|
||||
// carry A, or the one that could is past its own bound, B is stopped, saying why.
|
||||
//
|
||||
// The shape is the live group fix/assign-says-why-a-module-is-not-there as mesh-delivery showed it on
|
||||
// 2026-10-11 (issue 442's test): the controller's member superseded first in the order, the lab's after it.
|
||||
func TestAMemberAfterACarriableSupersededOneWaitsForItsCarrier(t *testing.T) {
|
||||
const ctl, lab, group = "novox/mesh-controller", "novox/mesh-lab", "fix/assign-says-why-a-module-is-not-there"
|
||||
at := time.Date(2026, 10, 7, 19, 4, 10, 0, time.UTC)
|
||||
modules := []string{"build-agent", "mesh-controller", "route-proxy"}
|
||||
const mergedA, mergedB, mergedC = "a5f53ef9ebdf3650", "77777777aaaaaaaa", "3f68a49500000000"
|
||||
aID, bID, cID := IDOf(ctl, "8adb7f1a051941e9"), IDOf(lab, "33e63ee5b47a7552"), IDOf(ctl, "3f68a495c0c0c0c0")
|
||||
|
||||
// A world in which A's walk was superseded by C's, C's walk runs, and B waits for its turn.
|
||||
open := func(t *testing.T) *world {
|
||||
a := &Delivery{ID: aID, Repository: ctl, Commit: "8adb7f1a051941e9", Number: 109, Base: "main", Branch: group,
|
||||
Group: group, State: Superseded, Since: at.Add(4 * time.Minute), Created: at.Add(-time.Hour), MergedAs: mergedA,
|
||||
MergedAt: at, Plan: planMoving(ctl, modules...), Check: &Verdict{Gate: "pass"},
|
||||
Walk: &WalkSeen{ID: "plan-a5f53e", Commit: mergedA, State: walkSuperseded, Branch: "main",
|
||||
Note: "superseded at tier 0 by plan-later (" + ctl + " at 3f68a495); planned there again"},
|
||||
Transitions: []Transition{{At: at.Add(4 * time.Minute), From: Delivering, To: Superseded, Event: EvSuperseded,
|
||||
Why: "a newer delivery to the same trunk took over its walk"}}}
|
||||
b := &Delivery{ID: bID, Repository: lab, Commit: "33e63ee5b47a7552", Number: 117, Base: "main", Branch: group,
|
||||
Group: group, State: Published, Since: at.Add(3 * time.Minute), Created: at.Add(-2 * time.Hour),
|
||||
MergedAs: mergedB, MergedAt: at.Add(2 * time.Minute), Plan: planMoving(lab, "mesh-lab"),
|
||||
Check: &Verdict{Gate: "pass"}, Walk: &WalkSeen{ID: "plan-lab", Commit: mergedB, State: walkBuilding,
|
||||
Waits: true, WaitedFor: "mesh-delivery", Branch: "main", Created: at.Add(3 * time.Minute)}}
|
||||
c := &Delivery{ID: cID, Repository: ctl, Commit: "3f68a495c0c0c0c0", Number: 110, Base: "main",
|
||||
Branch: "fix/later", State: Delivering, Since: at.Add(5 * time.Minute), Created: at.Add(-time.Hour),
|
||||
MergedAs: mergedC, MergedAt: at.Add(time.Minute), Plan: planMoving(ctl, modules...),
|
||||
Check: &Verdict{Gate: "pass"}, Walk: &WalkSeen{ID: "plan-later", Commit: mergedC, State: walkRolling,
|
||||
WaitedFor: "mesh-delivery", GoBy: byOwner, Branch: "main", Created: at.Add(4 * time.Minute)}}
|
||||
store := newMemStore()
|
||||
for _, d := range []*Delivery{a, b, c} {
|
||||
if err := store.PutDelivery(d); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
g := &Group{ID: group, Members: []string{aID, bID}, Closed: true, Order: []string{aID, bID},
|
||||
Said: "delivering", OrderOf: map[string]string{},
|
||||
Check: &GroupCheck{Asked: "check-1", At: at.Add(-time.Hour), Verdict: "pass", Summary: "both compose",
|
||||
Commits: commitsOf([]*Delivery{a, b})}}
|
||||
if err := store.PutGroup(g); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
ctlr := newFakeController()
|
||||
ctlr.put(Walk{ID: "plan-a5f53e", Repository: ctl, Branch: "main", Commit: mergedA, Created: at.Add(30 * time.Second),
|
||||
State: walkSuperseded, Note: a.Walk.Note, Modules: map[string]*WalkModule{}})
|
||||
ctlr.put(Walk{ID: "plan-lab", Repository: lab, Branch: "main", Commit: mergedB, Created: at.Add(3 * time.Minute),
|
||||
State: walkBuilding, Tiers: [][]string{{"mesh-lab"}}, Modules: map[string]*WalkModule{"mesh-lab": {}},
|
||||
Delivery: &WalkDelivery{Awaits: "mesh-delivery"}})
|
||||
went := at.Add(5 * time.Minute)
|
||||
later := Walk{ID: "plan-later", Repository: ctl, Branch: "main", Commit: mergedC, Created: at.Add(4 * time.Minute),
|
||||
State: walkRolling, Tiers: [][]string{modules}, Modules: map[string]*WalkModule{},
|
||||
Delivery: &WalkDelivery{Awaits: "mesh-delivery", Go: &went, By: byOwner}}
|
||||
for _, m := range modules {
|
||||
later.Modules[m] = &WalkModule{}
|
||||
}
|
||||
ctlr.put(later)
|
||||
w := newWorldOver(t, store, ctlr, newFakeForge(), at.Add(10*time.Minute))
|
||||
if err := w.h.Reconcile(); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
w.settleAll()
|
||||
return w
|
||||
}
|
||||
// end ends C's walk in the state given, every module sent to every machine running it.
|
||||
end := func(w *world, state string) {
|
||||
walk := w.ctl.walk("plan-later")
|
||||
sent := w.now
|
||||
for _, m := range modules {
|
||||
walk.Modules[m] = &WalkModule{State: "built", SentAt: &sent}
|
||||
}
|
||||
walk.State = state
|
||||
w.h.WalkMoved(w.ctl.put(walk))
|
||||
w.settleAll()
|
||||
}
|
||||
waits := func(t *testing.T, w *world) {
|
||||
t.Helper()
|
||||
if s := w.state(bID); s != Published || containsString(w.ctl.delivers, "plan-lab") {
|
||||
t.Fatalf("B is %s, let go %v; want it published and waiting while C's walk can still carry A", s, w.ctl.delivers)
|
||||
}
|
||||
g := w.h.groups[group]
|
||||
if state, why := GroupState(g, w.h.membersOf(g)); state != "delivering" {
|
||||
t.Fatalf("the group reads %s (%s), want delivering", state, why)
|
||||
}
|
||||
}
|
||||
stopped := func(t *testing.T, w *world, says string) {
|
||||
t.Helper()
|
||||
d := w.delivery(bID)
|
||||
if d.State != Stopped || len(d.Transitions) == 0 {
|
||||
t.Fatalf("B is %s, want stopped", d.State)
|
||||
}
|
||||
why := d.Transitions[len(d.Transitions)-1].Why
|
||||
if !strings.Contains(why, aID) || !strings.Contains(why, says) {
|
||||
t.Fatalf("B was stopped saying %q, want it to name %s and say %q", why, aID, says)
|
||||
}
|
||||
if containsString(w.ctl.delivers, "plan-lab") {
|
||||
t.Fatalf("B's walk was let go %v", w.ctl.delivers)
|
||||
}
|
||||
}
|
||||
|
||||
t.Run("a later walk that can carry A is open: B waits", func(t *testing.T) {
|
||||
waits(t, open(t))
|
||||
})
|
||||
t.Run("that walk delivered and covers A: B takes its turn", func(t *testing.T) {
|
||||
w := open(t)
|
||||
w.forge.contains[mergedA+" in "+mergedC] = true
|
||||
end(w, walkDone)
|
||||
if s := w.state(cID); s != Delivered {
|
||||
t.Fatalf("C is %s", s)
|
||||
}
|
||||
if d := w.delivery(aID); d.State != Superseded || d.CoveredBy == "" {
|
||||
t.Fatalf("A is %s, covered by %q", d.State, d.CoveredBy)
|
||||
}
|
||||
if s := w.state(bID); s != Delivering || !containsString(w.ctl.delivers, "plan-lab") {
|
||||
t.Fatalf("B is %s, let go %v; want its turn taken", s, w.ctl.delivers)
|
||||
}
|
||||
})
|
||||
t.Run("the forge says the later merge does not contain A: B is stopped", func(t *testing.T) {
|
||||
w := open(t)
|
||||
w.forge.contains[mergedA+" in "+mergedC] = false
|
||||
end(w, walkDone)
|
||||
stopped(t, w, "can carry")
|
||||
})
|
||||
t.Run("the later walk failed: B is stopped", func(t *testing.T) {
|
||||
w := open(t)
|
||||
end(w, walkFailed)
|
||||
if s := w.state(cID); s != Failed {
|
||||
t.Fatalf("C is %s", s)
|
||||
}
|
||||
stopped(t, w, "can carry")
|
||||
})
|
||||
t.Run("the later walk stays open past its own bound: B is stopped", func(t *testing.T) {
|
||||
w := open(t)
|
||||
w.later(Bounds[Delivering].For - 10*time.Minute)
|
||||
w.settleAll()
|
||||
waits(t, w)
|
||||
w.later(10 * time.Minute)
|
||||
w.settleAll()
|
||||
stopped(t, w, "past its bound")
|
||||
})
|
||||
t.Run("the forge never says whether the delivered walk contains A: B is stopped after the grace", func(t *testing.T) {
|
||||
w := open(t)
|
||||
end(w, walkDone)
|
||||
waits(t, w)
|
||||
w.later(containsGrace + time.Minute)
|
||||
w.settleAll()
|
||||
stopped(t, w, "did not say")
|
||||
})
|
||||
}
|
||||
Reference in New Issue
Block a user