From 6ae112d299d588c706caeb9fa32720e9815e1bbe Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 8 Oct 2026 10:43:21 +0200 Subject: [PATCH] Say which order rule won, and how to end a contradiction (hq issue 309) A group whose order rules disagreed was refused as "its order contradicts itself" with only the member ids: nobody could see which rules clashed or what to write to resolve it. The controller now resolves rules by precedence (hq ADR 0249) and answers, per pair, the rules it won over, and per contradiction no precedence resolves, both rules and how to declare the order. mesh-delivery keeps both, lists them in groups and the plan note, and refuses a group by those words; a cycle through more members names the pairs in it. An older controller's answer reads as before. --- .../cmd/mesh-delivery/effects.go | 5 +- .../mesh-delivery/cmd/mesh-delivery/holder.go | 30 +++++++++- .../cmd/mesh-delivery/holder_test.go | 58 +++++++++++++++++++ .../mesh-delivery/cmd/mesh-delivery/main.go | 3 +- .../mesh-delivery/cmd/mesh-delivery/model.go | 38 +++++++++--- .../mesh-delivery/cmd/mesh-delivery/ports.go | 10 ++-- .../mesh-delivery/cmd/mesh-delivery/verbs.go | 21 ++++--- 7 files changed, 140 insertions(+), 25 deletions(-) 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)) }