From c5a2edf04a84337d2d413378b0e6c1c024a5a2a7 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 8 Oct 2026 10:43:42 +0200 Subject: [PATCH] Resolve a group's order rules by precedence, not as a cycle (hq issue 309) The group feat/a-machine-joins-through-the-tunnel was refused: the controller's member moved the build agent, which builds the node-engine (built by: controller first), and the node-engine goes before the controller (engine before controller: engine first). Both rules applied to one pair in opposite directions, and every two-way pair was a cycle. Rules now have a precedence (hq ADR 0249): a declared after: line, then what the graph and the change say (built by, version skew), then the rollout default engine-before-controller. The higher rule decides the pair, which says what it won over. Rules of one rank both ways are still refused, as a contradiction naming both rules and how to declare the order. TestReplay309 replays the group with only what orderOf had before. --- cmd/mesh-controller/delivery.go | 141 +++++++++++++++++++++++--- cmd/mesh-controller/delivery_test.go | 78 +++++++++++++- cmd/mesh-controller/replay309_test.go | 50 +++++++++ internal/catalogue/verbs.go | 3 +- 4 files changed, 256 insertions(+), 16 deletions(-) create mode 100644 cmd/mesh-controller/replay309_test.go diff --git a/cmd/mesh-controller/delivery.go b/cmd/mesh-controller/delivery.go index cddb694f..4bffcb9c 100644 --- a/cmd/mesh-controller/delivery.go +++ b/cmd/mesh-controller/delivery.go @@ -168,11 +168,22 @@ func (m orderMember) pull() link.PullUpdated { ModuleDirs: m.ModuleDirs, ModuleDirsSaid: m.ModuleDirsSaid} } -// orderPair is one "before" among a group's members, and why. +// orderPair is one "before" among a group's members, why, and the rules it won over. type orderPair struct { Before string `json:"before"` After string `json:"after"` Why string `json:"why"` + // Over is the rules that ordered the pair the other way and lost to Why, by precedence (novox/hq ADR + // 0249): said, so a person reading the order sees which rule won. + Over []string `json:"over,omitempty"` +} + +// orderContradiction is two members that rules of one rank order both ways: no precedence resolves it, so +// the group is refused, the rules named, and Said says how a person declares the order. +type orderContradiction struct { + Members []string `json:"members"` + Rules []orderPair `json:"rules"` + Said string `json:"said"` } // orderReach is what a member moves, as the planner says. @@ -183,12 +194,14 @@ type orderReach struct { Manifests []string `json:"manifests,omitempty"` } -// groupOrder is a group's order: the members in it, every pair and why, and the cycle when there is one. +// groupOrder is a group's order: the members in it, every pair and why, and the cycle when there is one — +// with each contradiction no precedence resolves. type groupOrder struct { - Order []string `json:"order"` - Pairs []orderPair `json:"pairs,omitempty"` - Cycle []string `json:"cycle,omitempty"` - Reach map[string]orderReach `json:"reach,omitempty"` + Order []string `json:"order"` + Pairs []orderPair `json:"pairs,omitempty"` + Cycle []string `json:"cycle,omitempty"` + Contradictions []orderContradiction `json:"contradictions,omitempty"` + Reach map[string]orderReach `json:"reach,omitempty"` } // The reasons a pair is ordered, in the words the delivery plan shows. @@ -199,7 +212,23 @@ const ( orderEngineFirst = "engine before controller" ) -// orderOf is a group's order (novox/hq ADR 0239 decision 4), pure. A member goes before another when: +// orderRank is a rule's precedence when two rules order one pair both ways (novox/hq ADR 0249), the lower +// winning: what a person declared; then what the graph and the change say (built by, version skew); then +// the rollout default, which infers a direction from the modules' names alone. +func orderRank(why string) int { + switch why { + case orderDeclared: + return 0 + case orderBuiltBy, orderVersionSkew: + return 1 + case orderEngineFirst: + return 2 + } + return 3 +} + +// orderOf is a group's order (novox/hq ADR 0239 decision 4, ADR 0249), pure. A member goes before another +// when: // // - its pull request is named in the other's `after:` lines (declared); // - it moves a module the other's moved modules are built by, stand on, package or declare (built by); @@ -209,24 +238,28 @@ const ( // does not yet grant, and a controller sends nothing an older engine would refuse — ADR 0236's rollout // order). // -// Otherwise, by repository then id, so every reading gives one order. A pair both ways is a cycle: the -// members in it are named, and none of them is ordered. +// When rules order one pair both ways, the one of higher precedence wins (orderRank) and the pair says +// what it won over. Rules of one rank both ways are a contradiction: both are kept as pairs, the members +// are the cycle, and the contradiction says how to declare the order. Otherwise, by repository then id, so +// every reading gives one order. A cycle through three members or more is named by its members, and none +// of them is ordered. func orderOf(members []orderMember, reach map[string]orderReach, edges []inventory.Edge) groupOrder { out := groupOrder{Reach: reach} byID := map[string]orderMember{} for _, m := range members { byID[m.ID] = m } + var claims []orderPair add := func(before, after, why string) { if before == after { return } - for _, p := range out.Pairs { - if p.Before == before && p.After == after { + for _, p := range claims { + if p.Before == before && p.After == after && p.Why == why { return } } - out.Pairs = append(out.Pairs, orderPair{Before: before, After: after, Why: why}) + claims = append(claims, orderPair{Before: before, After: after, Why: why}) } named := func(said, repository string) bool { said = strings.TrimSuffix(strings.TrimSpace(said), ".git") @@ -263,6 +296,7 @@ func orderOf(members []orderMember, reach map[string]orderReach, edges []invento } } } + out.Pairs, out.Contradictions = resolveClaims(claims, byID) sort.Slice(out.Pairs, func(i, j int) bool { if out.Pairs[i].Before != out.Pairs[j].Before { return out.Pairs[i].Before < out.Pairs[j].Before @@ -312,6 +346,89 @@ func orderOf(members []orderMember, reach map[string]orderReach, edges []invento return out } +// orderRules is the rules in the order a pair names them when more than one says the same. +var orderRules = []string{orderDeclared, orderBuiltBy, orderVersionSkew, orderEngineFirst} + +// resolveClaims is the pairs the rules' claims come to (novox/hq ADR 0249): for each two members, the claims +// of the highest precedence decide, and the claims they overrule are said on the pair. Claims of one rank +// both ways are a contradiction: both pairs are kept, so the walk finds the cycle, and the contradiction +// says how a person declares the order. +func resolveClaims(claims []orderPair, byID map[string]orderMember) ([]orderPair, []orderContradiction) { + byTwo := map[[2]string][]orderPair{} + var keys [][2]string + for _, c := range claims { + k := [2]string{min(c.Before, c.After), max(c.Before, c.After)} + if _, ok := byTwo[k]; !ok { + keys = append(keys, k) + } + byTwo[k] = append(byTwo[k], c) + } + slices.SortFunc(keys, func(a, b [2]string) int { + if c := strings.Compare(a[0], b[0]); c != 0 { + return c + } + return strings.Compare(a[1], b[1]) + }) + ruleIndex := func(why string) int { return slices.Index(orderRules, why) } + var pairs []orderPair + var contradictions []orderContradiction + for _, k := range keys { + cs := byTwo[k] + slices.SortStableFunc(cs, func(a, b orderPair) int { return ruleIndex(a.Why) - ruleIndex(b.Why) }) + top := orderRank(cs[0].Why) + var won []orderPair + ways := map[string]bool{} + for _, c := range cs { + if orderRank(c.Why) == top { + won = append(won, c) + ways[c.Before] = true + } + } + if len(ways) == 1 { + p := orderPair{Before: won[0].Before, After: won[0].After, Why: won[0].Why} + for _, c := range cs { + if c.Before != p.Before && !slices.Contains(p.Over, c.Why) { + p.Over = append(p.Over, c.Why) + } + } + pairs = append(pairs, p) + continue + } + // One rank both ways: no precedence resolves it. + var first, second *orderPair + var said []string + for i := range won { + c := won[i] + if c.Before == k[0] && first == nil { + first = &won[i] + } + if c.Before == k[1] && second == nil { + second = &won[i] + } + said = append(said, fmt.Sprintf("%s puts %s first", c.Why, c.Before)) + } + pairs = append(pairs, orderPair{Before: first.Before, After: first.After, Why: first.Why}, + orderPair{Before: second.Before, After: second.After, Why: second.Why}) + a, b := byID[k[0]], byID[k[1]] + how := fmt.Sprintf("neither outranks the other: declare the order with a line `after: %s` in %s's "+ + "description, or `after: %s` in %s's", a.Repository, pullName(b), b.Repository, pullName(a)) + if top == orderRank(orderDeclared) { + how = "each pull request declares it goes after the other: remove one of the `after:` lines" + } + contradictions = append(contradictions, orderContradiction{Members: []string{k[0], k[1]}, Rules: won, + Said: fmt.Sprintf("%s and %s: %s; %s", k[0], k[1], strings.Join(said, ", "), how)}) + } + return pairs, contradictions +} + +// pullName is a member as a person finds it: its pull request, or its id when no number was said. +func pullName(m orderMember) string { + if m.Number > 0 { + return fmt.Sprintf("%s#%d", m.Repository, m.Number) + } + return m.ID +} + // reachOfMembers is each member's reach, as the planner says it. func reachOfMembers(members []orderMember, entries []inventory.Entry, read map[string][]inventory.ReadRepository, edges []inventory.Edge) map[string]orderReach { diff --git a/cmd/mesh-controller/delivery_test.go b/cmd/mesh-controller/delivery_test.go index 6f685497..aeccc111 100644 --- a/cmd/mesh-controller/delivery_test.go +++ b/cmd/mesh-controller/delivery_test.go @@ -3,6 +3,7 @@ package main import ( "encoding/json" "reflect" + "slices" "strings" "testing" "time" @@ -63,11 +64,82 @@ func TestAGroupIsOrderedByTheGraphAndWhatItsPullRequestsSay(t *testing.T) { t.Fatalf("another reading ordered %v", again.Order) } - // A declared order against an inferred one is a cycle: named, and nothing ordered. + // A declared order against an inferred one wins (novox/hq ADR 0249), and the pair says what it won over. members[1].After = []string{"mesh-catalog"} got = orderOf(members, reach, edges) - if !reflect.DeepEqual(got.Cycle, []string{"cat", "ctl"}) { - t.Fatalf("the cycle is %v, ordered %v", got.Cycle, got.Order) + if len(got.Cycle) > 0 || len(got.Contradictions) > 0 { + t.Fatalf("a declared order did not win: cycle %v, %+v", got.Cycle, got.Contradictions) + } + if want := []string{"agent", "lab", "app", "cat", "host", "ctl"}; !reflect.DeepEqual(got.Order, want) { + t.Fatalf("ordered %v, wanted %v", got.Order, want) + } + if !slices.ContainsFunc(got.Pairs, func(p orderPair) bool { + return p.Before == "cat" && p.After == "ctl" && p.Why == orderDeclared && reflect.DeepEqual(p.Over, []string{orderVersionSkew}) + }) || slices.ContainsFunc(got.Pairs, func(p orderPair) bool { return p.Before == "ctl" && p.After == "cat" }) { + t.Fatalf("the declared pair does not say it won over version skew: %+v", got.Pairs) + } + + // Declared both ways is a contradiction no precedence resolves: a cycle, and how to end it. + members[0].After = []string{"novox/mesh-controller"} + got = orderOf(members, reach, edges) + if !reflect.DeepEqual(got.Cycle, []string{"cat", "ctl"}) || len(got.Contradictions) != 1 { + t.Fatalf("the cycle is %v, ordered %v, contradictions %+v", got.Cycle, got.Order, got.Contradictions) + } + if c := got.Contradictions[0]; !reflect.DeepEqual(c.Members, []string{"cat", "ctl"}) || + !strings.Contains(c.Said, "remove one of the `after:` lines") { + t.Fatalf("the contradiction is said %+v", c) + } +} + +// novox/hq ADR 0249: when two rules order one pair both ways, the rule of higher precedence wins and the +// pair says what it overruled; rules of one rank both ways are refused, naming both and how to declare. +func TestAnOrderRuleYieldsToAStrongerOne(t *testing.T) { + members := []orderMember{ + {ID: "ctl", Repository: "novox/mesh-controller", Number: 7}, + {ID: "host", Repository: "novox/mesh-host", Number: 9}, + } + reach := map[string]orderReach{ + "ctl": {Moved: []string{"mesh-controller", "build-agent"}}, + "host": {Moved: []string{"mesh-host"}}, + } + builtBy := []inventory.Edge{{From: "mesh-host", To: "build-agent", Kind: inventory.EdgeBuiltBy}} + + // Built by outranks engine before controller. + got := orderOf(members, reach, builtBy) + if !reflect.DeepEqual(got.Order, []string{"ctl", "host"}) || len(got.Cycle) > 0 { + t.Fatalf("ordered %v, cycle %v", got.Order, got.Cycle) + } + if want := []orderPair{{Before: "ctl", After: "host", Why: orderBuiltBy, Over: []string{orderEngineFirst}}}; !reflect.DeepEqual(got.Pairs, want) { + t.Fatalf("pairs %+v, wanted %+v", got.Pairs, want) + } + + // A declared line outranks both. + members[0].After = []string{"mesh-host"} + got = orderOf(members, reach, builtBy) + if !reflect.DeepEqual(got.Order, []string{"host", "ctl"}) || len(got.Cycle) > 0 || + !reflect.DeepEqual(got.Pairs[0].Over, []string{orderBuiltBy}) || got.Pairs[0].Why != orderDeclared { + t.Fatalf("a declared order did not win: %v %+v", got.Order, got.Pairs) + } + + // Without the build agent, engine before controller stands alone. + members[0].After = nil + got = orderOf(members, map[string]orderReach{"ctl": {Moved: []string{"mesh-controller"}}, "host": reach["host"]}, builtBy) + if !reflect.DeepEqual(got.Order, []string{"host", "ctl"}) || got.Pairs[0].Why != orderEngineFirst || len(got.Pairs[0].Over) > 0 { + t.Fatalf("engine before controller alone: %v %+v", got.Order, got.Pairs) + } + + // Built by both ways is one rank against itself: refused, both rules named, and how to declare. + both := append(builtBy, inventory.Edge{From: "build-agent", To: "mesh-host", Kind: inventory.EdgeStandsOn}) + got = orderOf(members, reach, both) + if !reflect.DeepEqual(got.Cycle, []string{"ctl", "host"}) || len(got.Order) > 0 || len(got.Contradictions) != 1 { + t.Fatalf("a contradiction was ordered: %v, cycle %v, %+v", got.Order, got.Cycle, got.Contradictions) + } + said := got.Contradictions[0].Said + for _, w := range []string{"built by puts ctl first", "built by puts host first", + "`after: novox/mesh-controller` in novox/mesh-host#9's description", "`after: novox/mesh-host` in novox/mesh-controller#7's"} { + if !strings.Contains(said, w) { + t.Errorf("the contradiction does not say %q: %s", w, said) + } } } diff --git a/cmd/mesh-controller/replay309_test.go b/cmd/mesh-controller/replay309_test.go new file mode 100644 index 00000000..f80b948e --- /dev/null +++ b/cmd/mesh-controller/replay309_test.go @@ -0,0 +1,50 @@ +package main + +import ( + "reflect" + "testing" + + "github.com/novox/mesh-controller/internal/inventory" +) + +// novox/hq issue 309, replayed with only what orderOf had before its fix, so it can be laid over the older +// commit. The group `feat/a-machine-joins-through-the-tunnel` (2026-10-08): the controller's pull request +// moved the controller and the build agent, the node-engine's moved the node-engine, the lab's moved +// nothing. The build agent builds the node-engine (built by: the controller's member first), and the +// node-engine goes before the controller (engine before controller: the node-engine's member first). Both +// rules applied to one pair in opposite directions, and the group was rejected as a cycle; the person merged +// the controller first by hand. A rule read from the graph outranks the rollout default, so the group is +// ordered, the controller's member first. +func TestReplay309(t *testing.T) { + members := []orderMember{ + {ID: "novox/mesh-controller@8bbfdb53db2b", Repository: "novox/mesh-controller", Number: 132}, + {ID: "novox/mesh-host@86cbebd10f8a", Repository: "novox/mesh-host", Number: 52}, + {ID: "novox/mesh-lab@e80a4b1642bf", Repository: "novox/mesh-lab", Number: 61}, + } + reach := map[string]orderReach{ + "novox/mesh-controller@8bbfdb53db2b": {Moved: []string{"build-agent", "mesh-controller", "route-proxy"}, + Manifests: []string{"module.json"}}, + "novox/mesh-host@86cbebd10f8a": {Moved: []string{"mesh-host"}}, + "novox/mesh-lab@e80a4b1642bf": {}, + } + // Every source-built module is built by the build seat's holder; the holder follows the controller. + edges := []inventory.Edge{ + {From: "build-agent", To: "mesh-controller", Kind: inventory.EdgeWorkerOf}, + {From: "mesh-controller", To: "build-agent", Kind: inventory.EdgeBuiltBy}, + {From: "mesh-host", To: "build-agent", Kind: inventory.EdgeBuiltBy}, + {From: "route-proxy", To: "build-agent", Kind: inventory.EdgeBuiltBy}, + } + got := orderOf(members, reach, edges) + if len(got.Cycle) > 0 { + t.Fatalf("the group is refused as a cycle %v; pairs %+v", got.Cycle, got.Pairs) + } + want := []string{"novox/mesh-controller@8bbfdb53db2b", "novox/mesh-host@86cbebd10f8a", "novox/mesh-lab@e80a4b1642bf"} + if !reflect.DeepEqual(got.Order, want) { + t.Fatalf("ordered %v, wanted %v; pairs %+v", got.Order, want, got.Pairs) + } + for _, p := range got.Pairs { + if p.Before == want[1] && p.After == want[0] { + t.Fatalf("a pair that lost still stands: %+v", p) + } + } +} diff --git a/internal/catalogue/verbs.go b/internal/catalogue/verbs.go index def34097..a6601ff6 100644 --- a/internal/catalogue/verbs.go +++ b/internal/catalogue/verbs.go @@ -145,7 +145,8 @@ var ControllerVerbs = []Verb{ "removed": "the files among paths it deletes, comma-separated"}, []string{"repository", "paths"})}, {Name: "delivery-order", Description: "A delivery group's order (novox/hq ADR 0239): its members in the order " + "they are delivered, every pair and why — declared, built by, version skew, engine before controller — and " + - "the cycle when the pairs contradict each other.", + "the rules each pair won over by precedence (ADR 0249); the cycle, and each contradiction no precedence " + + "resolves with how to declare the order, when the pairs contradict each other.", Input: schema(map[string]string{"members": "the members, as JSON: [{id, repository, base, head, number, paths, " + "module_dirs, module_dirs_said, removed, after}]"}, []string{"members"})}, {Name: "delivery-check", Description: "Ask the build seat for a delivery group's composed check (novox/hq ADR " +