diff --git a/modules/mesh-delivery/cmd/mesh-delivery/effects.go b/modules/mesh-delivery/cmd/mesh-delivery/effects.go index df3d428e..3736e280 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/effects.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/effects.go @@ -141,7 +141,10 @@ func ViewBody(d *Delivery, g *Group) string { if g != nil { fmt.Fprintf(&b, "**Group** `%s`, in order: %s\n", g.ID, strings.Join(g.Order, " → ")) for _, p := range g.Pairs { - fmt.Fprintf(&b, "- %s before %s: %s\n", p.Before, p.After, p.Why) + fmt.Fprintf(&b, "- %s\n", p.orderSaid()) + } + for _, c := range g.Contradictions { + fmt.Fprintf(&b, "- **refused**: %s\n", c.Said) } if g.Check != nil && g.Check.Verdict != "" { fmt.Fprintf(&b, "- composed together: %s — %s\n", g.Check.Verdict, g.Check.Summary) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index 4b6d0a84..f2cc2360 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -3,6 +3,7 @@ package main import ( "errors" "fmt" + "slices" "sort" "strings" "sync" @@ -925,7 +926,7 @@ func (h *Holder) planGroup(g *Group, members []*Delivery, now time.Time) []func( h.Logf("[mesh-delivery] group %s's order cannot be asked yet: %v", id, err) return } - g.Order, g.Pairs, g.Cycle, g.OrderOf = o.Order, o.Pairs, o.Cycle, commits + g.Order, g.Pairs, g.Cycle, g.Contradictions, g.OrderOf = o.Order, o.Pairs, o.Cycle, o.Contradictions, commits g.Check = nil // a check of another set of heads is no check of these g.Updated = h.Now() h.keepGroup(g) @@ -1086,7 +1087,7 @@ func GroupState(g *Group, members []*Delivery) (string, string) { case g.Closed: return "delivering", fmt.Sprintf("%d of %d delivered", count[Delivered], n) case len(g.Cycle) > 0: - return "rejected", "its order contradicts itself: " + strings.Join(g.Cycle, ", ") + return "rejected", cycleSaid(g) case count[Rejected] > 0: return "rejected", "a member's own check failed" case g.Check != nil && g.Check.Verdict != "" && g.Check.Verdict != "pass" && g.Check.Verdict != "warning": @@ -1125,3 +1126,28 @@ func containsString(xs []string, x string) bool { } return false } + +// cycleSaid is why a group's order is refused: each contradiction no precedence resolves, with the rules on +// both sides and how to declare the order — or, for a cycle through more members, the pairs in it. +func cycleSaid(g *Group) string { + why := "its order contradicts itself: " + if len(g.Contradictions) > 0 { + var said []string + for _, c := range g.Contradictions { + said = append(said, c.Said) + } + return why + strings.Join(said, "; ") + } + var pairs []string + for _, p := range g.Pairs { + if slices.Contains(g.Cycle, p.Before) && slices.Contains(g.Cycle, p.After) { + pairs = append(pairs, p.orderSaid()) + } + } + why += strings.Join(g.Cycle, ", ") + if len(pairs) > 0 { + why += " (" + strings.Join(pairs, "; ") + "); declare the order with a line `after: ` in " + + "the description of the pull request that goes later" + } + return why +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder_test.go b/modules/mesh-delivery/cmd/mesh-delivery/holder_test.go index 72727f08..c028a5d1 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder_test.go @@ -473,6 +473,64 @@ func TestAGroupWithACycleIsRejected(t *testing.T) { } } +// novox/hq issue 309: a group whose rules order one pair both ways at one rank is refused with both rules +// and how to declare the order; a pair a rule won by precedence says what it won over, and the group is +// checked as any other. +func TestAGroupSaysWhichOrderRuleWonOrWhyNoneDid(t *testing.T) { + w := newWorld(t) + said := "novox/a@a1a1a1a1a1a1 and novox/b@b2b2b2b2b2b2: built by puts novox/a@a1a1a1a1a1a1 first, built by puts " + + "novox/b@b2b2b2b2b2b2 first; neither outranks the other: declare the order with a line `after: novox/a` in " + + "novox/b#2's description, or `after: novox/b` in novox/a#1's" + w.ctl.order = func(ms []Member) Order { + return Order{Cycle: []string{ms[0].ID, ms[1].ID}, Contradictions: []Contradiction{{ + Members: []string{ms[0].ID, ms[1].ID}, Said: said, + Rules: []Pair{{Before: ms[0].ID, After: ms[1].ID, Why: "built by"}, {Before: ms[1].ID, After: ms[0].ID, Why: "built by"}}}}} + } + w.h.PullUpdated(pr("novox/a", 1, "a1a1a1a1a1a1a1", "feat/z")) + w.h.PullUpdated(pr("novox/b", 2, "b2b2b2b2b2b2b2", "feat/z")) + w.h.Checked(verdict("novox/a", 1, "a1a1a1a1a1a1a1", "pass")) + w.h.Checked(verdict("novox/b", 2, "b2b2b2b2b2b2b2", "pass")) + w.settleAll() + g := w.h.groups["feat/z"] + if state, why := GroupState(g, w.h.membersOf(g)); state != "rejected" || !strings.Contains(why, said) { + t.Fatalf("a contradiction was said %s: %s", state, why) + } + if lines := w.h.Groups(false); len(lines) != 1 || len(lines[0].Contradictions) != 1 { + t.Fatalf("groups does not list the contradiction: %+v", lines) + } + + // Resolved by precedence: ordered, the pair saying what it won over, and checked. + w2 := newWorld(t) + w2.ctl.order = func(ms []Member) Order { + return Order{Order: []string{ms[0].ID, ms[1].ID}, + Pairs: []Pair{{Before: ms[0].ID, After: ms[1].ID, Why: "built by", Over: []string{"engine before controller"}}}} + } + w2.h.PullUpdated(pr("novox/a", 1, "a1a1a1a1a1a1a1", "feat/z")) + w2.h.PullUpdated(pr("novox/b", 2, "b2b2b2b2b2b2b2", "feat/z")) + w2.h.Checked(verdict("novox/a", 1, "a1a1a1a1a1a1a1", "pass")) + w2.h.Checked(verdict("novox/b", 2, "b2b2b2b2b2b2b2", "pass")) + w2.settleAll() + g = w2.h.groups["feat/z"] + if len(g.Cycle) > 0 || len(w2.ctl.checks) != 1 { + t.Fatalf("a group ordered by precedence was not checked: cycle %v, checks %v", g.Cycle, w2.ctl.checks) + } + if got := g.Pairs[0].orderSaid(); !strings.HasSuffix(got, ": built by, over engine before controller") { + t.Fatalf("the pair is said %q", got) + } +} + +// A cycle through members with no contradiction of its own names the pairs in it. +func TestACycleWithoutAContradictionNamesItsPairs(t *testing.T) { + g := &Group{Cycle: []string{"a", "b", "c"}, Pairs: []Pair{{Before: "a", After: "b", Why: "built by"}, + {Before: "b", After: "c", Why: "declared"}, {Before: "c", After: "a", Why: "version skew"}}} + why := cycleSaid(g) + for _, w := range []string{"a before b: built by", "b before c: declared", "c before a: version skew", "`after: `"} { + if !strings.Contains(why, w) { + t.Errorf("the cycle does not say %q: %s", w, why) + } + } +} + // Stalled: a delivery past its state's bound is listed with what H2 may do; close takes only that. func TestStalledAndCloseWorkFromTheTable(t *testing.T) { w := newWorld(t) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/main.go b/modules/mesh-delivery/cmd/mesh-delivery/main.go index 8a14bd1b..c3c97b2a 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/main.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/main.go @@ -203,7 +203,8 @@ func tools(h *Holder, l *listening) []stdio.Tool { "required": []string{"id"}}, Run: func(a map[string]any) (any, error) { return h.Show(strArg(a, "id")) }}, {Name: seat + "groups", - Description: "Every delivery group: its members in order, why each pair is ordered, its composed check " + + Description: "Every delivery group: its members in order, why each pair is ordered and the rules it won " + + "over, each contradiction no precedence resolves with how to declare the order, its composed check " + "and its state, derived from its members.", Input: map[string]any{"type": "object", "properties": map[string]any{ "all": map[string]any{"type": "string", "enum": []string{"true", "false"}}}}, diff --git a/modules/mesh-delivery/cmd/mesh-delivery/model.go b/modules/mesh-delivery/cmd/mesh-delivery/model.go index 6d51cba6..4239c96e 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/model.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/model.go @@ -399,10 +399,13 @@ type Group struct { Updated time.Time `json:"updated"` // Order is the members in the order they are delivered, Pairs why, Cycle the members whose order // contradicts itself; OrderOf the member commits the order was worked out for. - Order []string `json:"order,omitempty"` - Pairs []Pair `json:"pairs,omitempty"` - Cycle []string `json:"cycle,omitempty"` - OrderOf map[string]string `json:"order_of,omitempty"` + Order []string `json:"order,omitempty"` + Pairs []Pair `json:"pairs,omitempty"` + Cycle []string `json:"cycle,omitempty"` + // Contradictions is each two members whose rules order them both ways at one rank, said with how a + // person declares the order (novox/hq ADR 0249). + Contradictions []Contradiction `json:"contradictions,omitempty"` + OrderOf map[string]string `json:"order_of,omitempty"` // Check is its composed check: asked of the controller for these member commits, and its verdict. Check *GroupCheck `json:"check,omitempty"` // Closed is set once a member reached the trunk: a group delivering takes no new member. @@ -412,11 +415,30 @@ type Group struct { Owed []Effect `json:"owed,omitempty"` } -// Pair is one "before" among a group's members. +// Pair is one "before" among a group's members: the rule that won, and the rules that ordered it the other +// way and lost by precedence (novox/hq ADR 0249). type Pair struct { - Before string `json:"before"` - After string `json:"after"` - Why string `json:"why"` + Before string `json:"before"` + After string `json:"after"` + Why string `json:"why"` + Over []string `json:"over,omitempty"` +} + +// Contradiction is two members that rules of one rank order both ways, which no precedence resolves: the +// rules, and Said, which names them and says how to declare the order. +type Contradiction struct { + Members []string `json:"members"` + Rules []Pair `json:"rules"` + Said string `json:"said"` +} + +// orderSaid is a pair as a person reads it: what won, and over what. +func (p Pair) orderSaid() string { + s := p.Before + " before " + p.After + ": " + p.Why + if len(p.Over) > 0 { + s += ", over " + strings.Join(p.Over, ", ") + } + return s } // GroupCheck is a group's composed check. diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ports.go b/modules/mesh-delivery/cmd/mesh-delivery/ports.go index 1de9f854..8b775503 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ports.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ports.go @@ -40,11 +40,13 @@ func MemberOf(d *Delivery) Member { CloneURL: d.CloneURL, After: d.After} } -// Order is a group's order as the controller works it out. +// Order is a group's order as the controller works it out: each pair with the rules it won over by +// precedence, and each contradiction no precedence resolves (novox/hq ADR 0249). type Order struct { - Order []string `json:"order"` - Pairs []Pair `json:"pairs,omitempty"` - Cycle []string `json:"cycle,omitempty"` + Order []string `json:"order"` + Pairs []Pair `json:"pairs,omitempty"` + Cycle []string `json:"cycle,omitempty"` + Contradictions []Contradiction `json:"contradictions,omitempty"` } // Controller is the controller's verbs this owner asks with. diff --git a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go index aaf01f0f..72f627d9 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/verbs.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/verbs.go @@ -107,14 +107,16 @@ func (h *Holder) Show(id string) (any, error) { // GroupLine is one group as `groups` lists it. type GroupLine struct { - ID string `json:"id"` - State string `json:"state"` - Why string `json:"why"` - Order []string `json:"order"` - Pairs []Pair `json:"pairs,omitempty"` - Cycle []string `json:"cycle,omitempty"` - Check string `json:"composed,omitempty"` - Members []string `json:"members"` + ID string `json:"id"` + State string `json:"state"` + Why string `json:"why"` + Order []string `json:"order"` + Pairs []Pair `json:"pairs,omitempty"` + Cycle []string `json:"cycle,omitempty"` + // Contradictions is each contradiction no precedence resolves, said with how to declare the order. + Contradictions []Contradiction `json:"contradictions,omitempty"` + Check string `json:"composed,omitempty"` + Members []string `json:"members"` } // Groups is the `groups` verb. @@ -131,7 +133,8 @@ func (h *Holder) Groups(all bool) []GroupLine { if !all && (state == "delivered" || state == "failed" || state == "stopped") { continue } - l := GroupLine{ID: g.ID, State: state, Why: why, Order: g.Order, Pairs: g.Pairs, Cycle: g.Cycle} + l := GroupLine{ID: g.ID, State: state, Why: why, Order: g.Order, Pairs: g.Pairs, Cycle: g.Cycle, + Contradictions: g.Contradictions} for _, d := range members { l.Members = append(l.Members, fmt.Sprintf("%s (%s)", d.ID, d.State)) }