From 8ca4b043216ffe271654669ee6bf932731ad15e4 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 8 Oct 2026 22:15:20 +0200 Subject: [PATCH 1/3] Hold the delivery planner to its recorded rules with a table and a property MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A table of merges (two repositories, every edge kind, a diamond, a cycle, files no build reads) and a seeded property over 500 random catalogues pin what a merge moves and in which tiers, per ADR 0162 and ADR 0238 §3. The shared-repository rows document today's behaviour that issue 338 would change, once with hand edges and once with edges derived from the store. The cycle check fails on main: hasCycle reads a packages edge between two modules of the last tier as a cycle, so a plan with none is said to have one. Left failing, marked BUG, for the planner's fix. --- cmd/mesh-controller/planner_records_test.go | 131 ++++++ cmd/mesh-controller/planner_rules_test.go | 453 ++++++++++++++++++++ 2 files changed, 584 insertions(+) create mode 100644 cmd/mesh-controller/planner_records_test.go create mode 100644 cmd/mesh-controller/planner_rules_test.go diff --git a/cmd/mesh-controller/planner_records_test.go b/cmd/mesh-controller/planner_records_test.go new file mode 100644 index 00000000..bb636f13 --- /dev/null +++ b/cmd/mesh-controller/planner_records_test.go @@ -0,0 +1,131 @@ +package main + +import ( + "reflect" + "strings" + "testing" + "time" + + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/inventory" +) + +// **The planner over edges the store derives**, not edges written by hand: modules registered, builds +// recorded with what they stood on and which repositories they read, the relation answered by +// inventory.Dependencies (dependenciesOf over the records), and the merge planned by reachOfMerge — the +// path a real merge takes, short of the bus. +// +// **The repository rows are CURRENT BEHAVIOUR, documented — not the rule the operator states** +// (novox/hq issue 338, and the decision pending on it): a build that read a repository gives its module a +// packages edge to every module built from that repository, and mergeCandidates moves it on any merge to +// that repository, whatever the files. So a change to C alone, or to a README, moves the module that +// packages C's repository. ADR 0238 §3 records exactly that today ("a repository a recipe names"); the +// expectations marked 338 change with that decision. +func TestASharedRepositoryIsPlannedFromTheRecordsAsItIsToday(t *testing.T) { + inv := inventory.ForTest(t) + ctx := t.Context() + asked := time.Now().Add(-time.Hour) + register := func(m catalogue.Manifest, repository, path string, against []string, read []inventory.ReadRepository) { + t.Helper() + if err := inv.RegisterModule(ctx, m, inventory.Source{Repository: repository, Seat: "git", Path: path, + Ref: "main", BuiltFrom: "old", Asked: asked}); err != nil { + t.Fatal(err) + } + if err := inv.RecordBuild(ctx, inventory.Build{ID: "build-" + m.Module, Repository: repository, Ref: "main", + Module: m.Module, Commit: "old", On: "builder", Path: path, Against: against, Read: read, Asked: asked}); err != nil { + t.Fatal(err) + } + } + controllerRead := []inventory.ReadRepository{{Repository: "novox/mesh-controller", Ref: "main"}} + agent := catalogue.Manifest{Module: "build-agent", Version: "1", + Claims: []catalogue.Claim{{Name: "node-build-agent", Scope: catalogue.ScopeNode}}} + + // The shape of issue 338. + register(catalogue.Manifest{Module: "mesh-controller", Version: "1"}, "novox/mesh-controller", "", nil, nil) + register(agent, "novox/mesh-catalog", "modules/build-agent", nil, controllerRead) + register(catalogue.Manifest{Module: "route-proxy", Version: "1"}, "novox/mesh-catalog", "modules/route-proxy", nil, controllerRead) + register(catalogue.Manifest{Module: "gitea", Version: "1"}, "novox/mesh-catalog", "modules/gitea", nil, nil) + // A, B and C in one repository; D built against A's artifact, E declaring B, P packaging the repository. + for _, n := range []string{"a", "b", "c"} { + register(catalogue.Manifest{Module: n, Version: "1"}, "novox/one", "modules/"+n, nil, nil) + } + register(catalogue.Manifest{Module: "d", Version: "1"}, "novox/two", "d", + []string{catalogue.ArtifactStoreScheme + "a/runtime@sha256:" + strings.Repeat("0", 64)}, nil) + register(catalogue.Manifest{Module: "e", Version: "1", Build: &catalogue.Build{ + On: []catalogue.BuildsOn{{Arg: "BASE", Module: "b", Artifact: "runtime"}}}}, "novox/two", "e", nil, nil) + register(catalogue.Manifest{Module: "p", Version: "1"}, "novox/two", "p", nil, + []inventory.ReadRepository{{Repository: "novox/one", Ref: "main"}}) + + entries, err := inv.Catalogued(ctx) + if err != nil { + t.Fatal(err) + } + read, err := inv.ReadRepositories(ctx) + if err != nil { + t.Fatal(err) + } + edges, err := inv.Dependencies(ctx) + if err != nil { + t.Fatal(err) + } + + // The edges the hand-written rows of TestASharedRepositoryMovesWhatPackagesItAsItDoesToday use are + // the ones derived here. + var shared []inventory.Edge + in338 := map[string]bool{"mesh-controller": true, "build-agent": true, "route-proxy": true, "gitea": true} + for _, e := range edges { + if in338[e.From] && in338[e.To] { + shared = append(shared, e) + } + } + if !reflect.DeepEqual(shared, sharedRepositoryEdges) { + t.Errorf("derived %v\nthe hand-written rows use %v", shared, sharedRepositoryEdges) + } + // Each kind derived from its record: built against (stands-on), build.on (declared), read (packages). + for _, want := range []inventory.Edge{ + dep("d", inventory.EdgeStandsOn, "a"), + dep("e", inventory.EdgeDeclared, "b"), + dep("p", inventory.EdgePackages, "a"), + dep("p", inventory.EdgePackages, "b"), + dep("p", inventory.EdgePackages, "c"), + dep("d", inventory.EdgeBuiltBy, "build-agent"), + } { + found := false + for _, e := range edges { + found = found || e == want + } + if !found { + t.Errorf("no %s %s %s derived: %v", want.From, want.Kind, want.To, edges) + } + } + + for _, c := range []struct { + what, repo string + paths []string + want string + issue338 bool + }{ + {"A and B changed, C untouched: D after A, E after B; P packages their repository", "one", + []string{"modules/a/x.go", "modules/b/x.go"}, "a,b,p | d,e", false}, + {"C alone: C, and P, which packages C's repository", "one", + []string{"modules/c/x.go"}, "c,p", true}, + {"a README of the repository P packages: P moves, nothing built from it does", "one", + []string{"README.md"}, "p", true}, + {"the dependent's repository: D alone", "two", []string{"d/main.go"}, "d", false}, + {"a README of the controller's repository: all three, three tiers", "mesh-controller", + []string{"README.md"}, "mesh-controller | build-agent | route-proxy", true}, + {"the route proxy's directory in the catalogue: it alone", "mesh-catalog", + []string{"modules/route-proxy/module.json"}, "route-proxy", false}, + {"the build agent's directory: it alone, nothing it builds", "mesh-catalog", + []string{"modules/build-agent/module.json"}, "build-agent", false}, + } { + _, got := planMerge(t, c.repo, c.paths, entries, read, edges) + if got != c.want { + tag := "" + if c.issue338 { + tag = " (current behaviour, issue 338)" + } + t.Errorf("%s: planned %q, wanted %q%s", c.what, got, c.want, tag) + } + } +} diff --git a/cmd/mesh-controller/planner_rules_test.go b/cmd/mesh-controller/planner_rules_test.go new file mode 100644 index 00000000..f6c0c5cf --- /dev/null +++ b/cmd/mesh-controller/planner_rules_test.go @@ -0,0 +1,453 @@ +package main + +import ( + "fmt" + "math/rand/v2" + "sort" + "strings" + "testing" + + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// The delivery planner's rules, as recorded (novox/hq ADR 0162 §1, ADR 0238 §3), held by one table and +// one property over reachOfMerge — the planner's one answer to "what does this merge move, and in which +// order". A row that fails here on main is a planner that breaks a recorded rule: the row stays, the +// expectation is not bent to the code. +// +// The kinds of edge, as the code reads them (release_plan.go): +// +// kind widens the plan orders the tiers +// stands-on yes yes, after its base is built +// declared yes yes, after its base is built +// packages yes no, the same tier (a code dependency) +// built-by no yes, after the build machine — except for what the build machine stands +// on, and for the controller whose worker it binds +// worker-of no yes, the build seat's holder after the controller (hq issue 206) + +const ( + repoOne = "http://forge.internal:20000/novox/one.git" + repoTwo = "http://forge.internal:20000/novox/two.git" +) + +// dep is one edge of the catalogue's relation: from depends on to, in the way kind says. +func dep(from, kind, to string) inventory.Edge { + return inventory.Edge{From: from, To: to, Kind: kind} +} + +// tiered is a plan's tiers as one line: a tier's modules by comma, tiers by " | ". Empty for no plan. +func tiered(tiers [][]string) string { + var out []string + for _, t := range tiers { + out = append(out, strings.Join(t, ",")) + } + return strings.Join(out, " | ") +} + +// planMerge is what reachOfMerge plans for a merge of these files into a repository's main: its tiers as +// one line, and the files no build reads. It also holds the plan to its own shape: every module it builds +// is in exactly one tier. +func planMerge(t *testing.T, repo string, paths []string, entries []inventory.Entry, + read map[string][]inventory.ReadRepository, edges []inventory.Edge) (mergeReach, string) { + t.Helper() + m := link.SourceMoved{Owner: "novox", Repo: repo, Base: "main", Commit: "head", Paths: paths} + r := reachOfMerge(m, entries, read, edges) + seen := map[string]int{} + for _, tier := range r.Plan.Tiers { + for _, name := range tier { + seen[name]++ + } + } + for name := range r.Plan.Modules { + if seen[name] != 1 { + t.Errorf("%s/%v: %s is in %d tiers of %v", repo, paths, name, seen[name], r.Plan.Tiers) + } + } + if len(seen) != len(r.Plan.Modules) { + t.Errorf("%s/%v: the tiers %v hold modules the plan does not (%d)", repo, paths, r.Plan.Tiers, len(r.Plan.Modules)) + } + return r, tiered(r.Plan.Tiers) +} + +// **A merge moves the modules whose directory it changed, and everything built on them; nothing else.** +// Each row is a catalogue (modules in directories of one or two repositories), its dependencies with +// their kinds, the files one commit changed, and the exact plan: the moved set and its tier order. +func TestAPlanIsWhatTheChangeTouchedAndWhatIsBuiltOnIt(t *testing.T) { + in := func(repo, dir string, names ...string) []inventory.Entry { + var out []inventory.Entry + for _, n := range names { + d := dir + "/" + n + if dir == "" { + d = n + } + out = append(out, fromRepo(n, repo, d)) + } + return out + } + // Modules a to f, x and z in novox/one under modules/; g in novox/two at g/. + entries := append(in(repoOne, "modules", "a", "b", "c", "d", "e", "f", "x", "z"), in(repoTwo, "", "g")...) + const ( + standsOn = inventory.EdgeStandsOn + declared = inventory.EdgeDeclared + packages = inventory.EdgePackages + builtBy = inventory.EdgeBuiltBy + workerOf = inventory.EdgeWorkerOf + ) + // The two real cycles the kinds resolve themselves (ADR 0162 §1, hq issue 206). + runtime := append(in(repoOne, "modules", "runtime", "builder"), fromRepo("controller", repoTwo, "")) + for _, c := range []struct { + what string + entries []inventory.Entry + edges []inventory.Edge + repo string + paths []string + want string // the tiers, " | " between them + unread string + cycle bool + }{ + // The operator's case: two modules changed, a third beside them in the same repository untouched, + // each changed one with a dependent. + {what: "A and B changed, C untouched beside them, D on A and E on B", + edges: []inventory.Edge{dep("d", standsOn, "a"), dep("e", declared, "b")}, + repo: "one", paths: []string{"modules/a/main.go", "modules/b/module.json"}, want: "a,b | d,e"}, + {what: "only A's directory: A and what stands on it", + edges: []inventory.Edge{dep("d", standsOn, "a"), dep("e", declared, "b")}, + repo: "one", paths: []string{"modules/a/main.go"}, want: "a | d"}, + {what: "only C's directory: C alone, nothing is built on it", + edges: []inventory.Edge{dep("d", standsOn, "a"), dep("e", declared, "b")}, + repo: "one", paths: []string{"modules/c/Dockerfile"}, want: "c"}, + {what: "only the dependent changed: its base does not move", + edges: []inventory.Edge{dep("d", standsOn, "a")}, + repo: "one", paths: []string{"modules/d/main.go"}, want: "d"}, + {what: "transitive: F on D on A, A changed", + edges: []inventory.Edge{dep("f", standsOn, "d"), dep("d", standsOn, "a")}, + repo: "one", paths: []string{"modules/a/x"}, want: "a | d | f"}, + {what: "transitive across kinds: F declared on D, D packages A", + edges: []inventory.Edge{dep("f", declared, "d"), dep("d", packages, "a")}, + repo: "one", paths: []string{"modules/a/x"}, want: "a,d | f"}, + + // Each kind alone: X depends on A, A changed (widening), then both changed (ordering). + {what: "stands-on (built against A's artifact) widens", edges: []inventory.Edge{dep("x", standsOn, "a")}, + repo: "one", paths: []string{"modules/a/x"}, want: "a | x"}, + {what: "stands-on orders", edges: []inventory.Edge{dep("x", standsOn, "a")}, + repo: "one", paths: []string{"modules/a/x", "modules/x/y"}, want: "a | x"}, + {what: "declared (build.on) widens", edges: []inventory.Edge{dep("x", declared, "a")}, + repo: "one", paths: []string{"modules/a/x"}, want: "a | x"}, + {what: "declared orders", edges: []inventory.Edge{dep("x", declared, "a")}, + repo: "one", paths: []string{"modules/a/x", "modules/x/y"}, want: "a | x"}, + {what: "packages widens, into the same tier", edges: []inventory.Edge{dep("x", packages, "a")}, + repo: "one", paths: []string{"modules/a/x"}, want: "a,x"}, + {what: "packages does not order", edges: []inventory.Edge{dep("x", packages, "a")}, + repo: "one", paths: []string{"modules/a/x", "modules/x/y"}, want: "a,x"}, + {what: "built-by never widens", edges: []inventory.Edge{dep("x", builtBy, "a")}, + repo: "one", paths: []string{"modules/a/x"}, want: "a"}, + {what: "built-by orders", edges: []inventory.Edge{dep("x", builtBy, "a")}, + repo: "one", paths: []string{"modules/a/x", "modules/x/y"}, want: "a | x"}, + {what: "worker-of never widens", edges: []inventory.Edge{dep("x", workerOf, "a")}, + repo: "one", paths: []string{"modules/a/x"}, want: "a"}, + {what: "worker-of orders", edges: []inventory.Edge{dep("x", workerOf, "a")}, + repo: "one", paths: []string{"modules/a/x", "modules/x/y"}, want: "a | x"}, + {what: "a change to the base alone does not move what it builds", edges: []inventory.Edge{dep("x", builtBy, "a"), + dep("d", builtBy, "a"), dep("e", standsOn, "a")}, + repo: "one", paths: []string{"modules/a/module.json"}, want: "a | e"}, + + // The cycles the kinds resolve: the build machine stands on the runtime image the runtime image is + // built by; the build seat's holder follows the controller that is built by it. + {what: "the build machine's base comes first, built by the build machine that runs", entries: runtime, + edges: []inventory.Edge{dep("runtime", builtBy, "builder"), dep("builder", standsOn, "runtime")}, + repo: "one", paths: []string{"modules/runtime/Dockerfile", "modules/builder/main.go"}, want: "runtime | builder"}, + {what: "the runtime image alone takes the build machine on it along", entries: runtime, + edges: []inventory.Edge{dep("runtime", builtBy, "builder"), dep("builder", standsOn, "runtime")}, + repo: "one", paths: []string{"modules/runtime/Dockerfile"}, want: "runtime | builder"}, + {what: "the build machine alone moves alone", entries: runtime, + edges: []inventory.Edge{dep("runtime", builtBy, "builder"), dep("builder", standsOn, "runtime")}, + repo: "one", paths: []string{"modules/builder/main.go"}, want: "builder"}, + {what: "the build seat's holder follows the controller it binds the worker of", entries: runtime, + edges: []inventory.Edge{dep("controller", builtBy, "builder"), dep("builder", workerOf, "controller")}, + repo: "two", paths: []string{"cmd/main.go"}, want: "controller"}, + + // Two repositories. + {what: "a dependent in another repository follows its base", + edges: []inventory.Edge{dep("g", standsOn, "a")}, + repo: "one", paths: []string{"modules/a/x"}, want: "a | g"}, + {what: "a directory of the same name in another repository is not this one's", + edges: []inventory.Edge{dep("g", standsOn, "a")}, + repo: "two", paths: []string{"modules/a/x"}, want: "", unread: "modules/a/x"}, + {what: "the dependent's own repository moves the dependent alone", + edges: []inventory.Edge{dep("g", standsOn, "a")}, + repo: "two", paths: []string{"g/main.go"}, want: "g"}, + + // A diamond. + {what: "a diamond, one side changed", edges: []inventory.Edge{dep("d", standsOn, "a"), dep("d", standsOn, "b")}, + repo: "one", paths: []string{"modules/a/x"}, want: "a | d"}, + {what: "a diamond, both sides changed", edges: []inventory.Edge{dep("d", standsOn, "a"), dep("d", standsOn, "b")}, + repo: "one", paths: []string{"modules/a/x", "modules/b/x"}, want: "a,b | d"}, + {what: "a diamond on one base", edges: []inventory.Edge{dep("d", standsOn, "a"), dep("d", declared, "b"), + dep("a", standsOn, "z"), dep("b", standsOn, "z")}, + repo: "one", paths: []string{"modules/z/x"}, want: "z | a,b | d"}, + {what: "a diamond of mixed kinds orders on the ordering side only", + edges: []inventory.Edge{dep("d", standsOn, "a"), dep("d", packages, "b")}, + repo: "one", paths: []string{"modules/b/x"}, want: "b,d"}, + + // A cycle the catalogue should never produce: what remains is one last tier, and said. + {what: "a cycle is one last tier, not lost", edges: []inventory.Edge{dep("a", standsOn, "b"), dep("b", standsOn, "a"), + dep("f", standsOn, "c")}, + repo: "one", paths: []string{"modules/a/x", "modules/c/x"}, want: "c | f | a,b", cycle: true}, + + // Files no build reads. + {what: "a README at the root of a repository whose modules all live below it", + edges: []inventory.Edge{dep("d", standsOn, "a")}, + repo: "one", paths: []string{"README.md"}, want: "", unread: "README.md"}, + {what: "a directory no module lives in", edges: []inventory.Edge{dep("d", standsOn, "a")}, + repo: "one", paths: []string{"modules/lib/x.go", "modules/README.md"}, want: "", + unread: "modules/lib/x.go,modules/README.md"}, + {what: "a module's directory beside a root file", edges: []inventory.Edge{dep("d", standsOn, "a")}, + repo: "one", paths: []string{"merge-check.sh", "modules/a/x"}, want: "a | d", unread: "merge-check.sh"}, + {what: "a directory whose name begins with a module's", edges: []inventory.Edge{dep("d", standsOn, "a")}, + repo: "one", paths: []string{"modules/ab/x"}, want: "", unread: "modules/ab/x"}, + } { + e := entries + if c.entries != nil { + e = c.entries + } + r, got := planMerge(t, c.repo, c.paths, e, nil, c.edges) + if got != c.want { + t.Errorf("%s: planned %q, wanted %q", c.what, got, c.want) + } + if u := strings.Join(r.Unread, ","); u != c.unread { + t.Errorf("%s: unread %q, wanted %q", c.what, u, c.unread) + } + // BUG found by this test on main (8170fc5): hasCycle reads any edge between two modules of the last + // tier as a cycle — a packages edge too, which puts both there by rule — so the merge handler logs + // "the last tier depends on itself … built together, in no order" for a plan with no cycle (the + // packages rows and the mixed diamond fail here). ADR 0162: a cycle is one last tier, *and said*; + // one said where there is none is the rule broken. The expectation stays. + if hasCycle(r.Plan.Tiers, c.edges) != c.cycle { + t.Errorf("%s: a cycle said %v, wanted %v (%v) [BUG: hasCycle counts a packages edge]", + c.what, !c.cycle, c.cycle, r.Plan.Tiers) + } + } +} + +// **CURRENT BEHAVIOUR, documented — not the rule the operator states.** novox/hq issue 338 (a module +// built from a shared repository moves on every merge to it) and the decision pending on it would change +// every row here. Today: +// +// - mesh-controller is built from its repository's root, so every file of that repository touches it; +// - route-proxy and build-agent package the whole of that repository (a build context), so the build +// record's `read` makes them move on any merge to it, whatever the files, and dependenciesOf gives +// each a packages edge to every module built from it; +// - built-by (route-proxy on build-agent) and worker-of (build-agent on the controller) make it three +// tiers. +// +// These follow ADR 0238 §3 as written ("the whole repository for a module built from its root, and a +// repository a recipe names"), so they are not failures; when the decision on issue 338 lands, these +// expectations change with it. The edges are the ones dependenciesOf derives from this catalogue — held +// to that by TestASharedRepositoryIsPlannedFromTheRecordsAsItIsToday, which derives them from the store. +func TestASharedRepositoryMovesWhatPackagesItAsItDoesToday(t *testing.T) { + const catalogueRepo = "http://forge.internal:20000/novox/mesh-catalog.git" + const controllerRepo = "http://forge.internal:20000/novox/mesh-controller.git" + entries := []inventory.Entry{ + fromRepo("mesh-controller", controllerRepo, ""), + fromRepo("build-agent", catalogueRepo, "modules/build-agent"), + fromRepo("route-proxy", catalogueRepo, "modules/route-proxy"), + fromRepo("gitea", catalogueRepo, "modules/gitea"), + } + read := map[string][]inventory.ReadRepository{ + "build-agent": {{Repository: "novox/mesh-controller", Ref: "main"}}, + "route-proxy": {{Repository: "novox/mesh-controller", Ref: "main"}}, + } + edges := sharedRepositoryEdges + for _, c := range []struct { + what, repo string + paths []string + want string + }{ + // The live three-tier plan of 2026-10-08 (issue 338). It is also the worker-of order (issue 206) + // that TestAMergeIsPlannedInTiersAlongTheThreeKindsOfDependency's controller case predates: that + // fixture has no worker-of edge and expects the builder first, then the controller and the proxy. + {"a README of the controller's repository moves all three, in three tiers", "mesh-controller", + []string{"README.md"}, "mesh-controller | build-agent | route-proxy"}, + {"the controller's own code: the same", "mesh-controller", + []string{"cmd/mesh-controller/main.go"}, "mesh-controller | build-agent | route-proxy"}, + {"the route proxy's program alone: the same, the controller with it", "mesh-controller", + []string{"examples/route-proxy/main.go"}, "mesh-controller | build-agent | route-proxy"}, + // In the catalogue, where they live, the rule is path-precise. + {"the route proxy's directory in the catalogue: it alone", "mesh-catalog", + []string{"modules/route-proxy/module.json"}, "route-proxy"}, + {"the build agent's directory: it alone, nothing it builds", "mesh-catalog", + []string{"modules/build-agent/module.json"}, "build-agent"}, + {"another module of the catalogue: neither", "mesh-catalog", + []string{"modules/gitea/index.ts"}, "gitea"}, + } { + r, got := planMerge(t, c.repo, c.paths, entries, read, edges) + if got != c.want { + t.Errorf("%s: planned %q, wanted %q (as today; issue 338)", c.what, got, c.want) + } + if c.repo == "mesh-controller" && strings.Join(r.Unread, ",") != "" { + t.Errorf("%s: a root-built module reads every file, and %v were said unread", c.what, r.Unread) + } + } +} + +// sharedRepositoryEdges is what dependenciesOf derives for the catalogue of the test above, sorted as it +// sorts them. +var sharedRepositoryEdges = []inventory.Edge{ + dep("build-agent", inventory.EdgePackages, "mesh-controller"), + dep("build-agent", inventory.EdgeWorkerOf, "mesh-controller"), + dep("gitea", inventory.EdgeBuiltBy, "build-agent"), + dep("mesh-controller", inventory.EdgeBuiltBy, "build-agent"), + dep("route-proxy", inventory.EdgeBuiltBy, "build-agent"), + dep("route-proxy", inventory.EdgePackages, "mesh-controller"), +} + +// **The planner's invariant, over random catalogues.** For any catalogue whose dependencies form no cycle +// and any set of changed files in one repository: +// +// - the plan is exactly the modules of that repository whose directory holds a changed file (every file, +// for a module built from the root), and everything reachable from them along stands-on, declared and +// packages — never along built-by or worker-of; +// - every stands-on, declared, built-by and worker-of edge with both ends in the plan has the module +// depended on in an earlier tier; +// - no cycle is said (this one fails on main: see the BUG note below). +// +// Seeded, so a failure is replayed by its seed and case. +func TestAPlanIsTheTouchedModulesAndWhatIsReachableAlongTheWideningEdges(t *testing.T) { + kinds := []string{inventory.EdgeStandsOn, inventory.EdgeDeclared, inventory.EdgePackages, + inventory.EdgeBuiltBy, inventory.EdgeWorkerOf} + widens := map[string]bool{inventory.EdgeStandsOn: true, inventory.EdgeDeclared: true, inventory.EdgePackages: true} + orders := map[string]bool{inventory.EdgeStandsOn: true, inventory.EdgeDeclared: true, + inventory.EdgeBuiltBy: true, inventory.EdgeWorkerOf: true} + // Directory names drawn from one pool, so two repositories hold directories of the same name, and one + // is a prefix of another. + dirs := []string{"a", "ab", "b", "c", "lib/x", "lib/y", "modules/a", "modules/a/sub"} + files := []string{"README.md", "merge-check.sh", "lib/z.go", "docs/x.md", "modules/README.md"} + + falseCycles, firstFalseCycle := 0, "" + for _, seed := range []uint64{1, 2, 3, 0x338, 0x162} { + rng := rand.New(rand.NewPCG(seed, seed^0x9e3779b97f4a7c15)) + for n := 0; n < 100; n++ { + repos := 1 + rng.IntN(3) + repoName := func(i int) string { return fmt.Sprintf("r%d", i) } + count := 1 + rng.IntN(12) + var entries []inventory.Entry + repoOf, dirOf := map[string]int{}, map[string]string{} + for i := 0; i < count; i++ { + name := fmt.Sprintf("m%02d", i) + repo := rng.IntN(repos) + dir := dirs[rng.IntN(len(dirs))] + if rng.IntN(12) == 0 { + dir = "" // built from the repository's root + } + repoOf[name], dirOf[name] = repo, dir + entries = append(entries, fromRepo(name, "http://forge.internal:20000/novox/"+repoName(repo)+".git", dir)) + } + // A graph with no cycle: a module depends only on modules made before it. + var edges []inventory.Edge + for i := 1; i < count; i++ { + for j := 0; j < i; j++ { + if rng.IntN(4) == 0 { + edges = append(edges, dep(fmt.Sprintf("m%02d", i), kinds[rng.IntN(len(kinds))], fmt.Sprintf("m%02d", j))) + } + } + } + merged := rng.IntN(repos) + var paths []string + for k := 1 + rng.IntN(4); k > 0; k-- { + if rng.IntN(3) == 0 { + paths = append(paths, files[rng.IntN(len(files))]) + } else { + paths = append(paths, dirs[rng.IntN(len(dirs))]+"/f.go") + } + } + + // What the rules say. + want := map[string]bool{} + for _, e := range entries { + name := e.Manifest.Module + if repoOf[name] != merged { + continue + } + for _, p := range paths { + if d := dirOf[name]; d == "" || p == d || strings.HasPrefix(p, d+"/") { + want[name] = true + } + } + } + for grew := true; grew; { + grew = false + for _, e := range edges { + if widens[e.Kind] && want[e.To] && !want[e.From] { + want[e.From], grew = true, true + } + } + } + + r, _ := planMerge(t, repoName(merged), paths, entries, nil, edges) + got := map[string]bool{} + tierOf := map[string]int{} + for i, tier := range r.Plan.Tiers { + for _, name := range tier { + got[name], tierOf[name] = true, i + } + } + replay := func() string { + return fmt.Sprintf("seed %#x case %d: repository %s, files %v\n modules %v\n edges %v\n tiers %v", + seed, n, repoName(merged), paths, describe(entries), edges, r.Plan.Tiers) + } + if !sameSet(got, want) { + t.Fatalf("planned %v, wanted %v\n%s", keys(got), keys(want), replay()) + } + for _, e := range edges { + if orders[e.Kind] && got[e.From] && got[e.To] && tierOf[e.To] >= tierOf[e.From] { + t.Fatalf("%s %s %s, and %s is in tier %d, not before %s's %d\n%s", e.From, e.Kind, e.To, + e.To, tierOf[e.To], e.From, tierOf[e.From], replay()) + } + } + if hasCycle(r.Plan.Tiers, edges) { + falseCycles++ + if firstFalseCycle == "" { + firstFalseCycle = replay() + } + } + } + } + // BUG found by this property on main (8170fc5); see the table's cycle check. Said once, after the + // other invariants have run over every case, so it hides none of them. + if falseCycles > 0 { + t.Errorf("a cycle said of a graph with none in %d of 500 cases [BUG: hasCycle counts a packages edge]; "+ + "the first:\n%s", falseCycles, firstFalseCycle) + } +} + +func sameSet(a, b map[string]bool) bool { + if len(a) != len(b) { + return false + } + for k := range a { + if !b[k] { + return false + } + } + return true +} + +func keys(m map[string]bool) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + sort.Strings(out) + return out +} + +func describe(entries []inventory.Entry) []string { + var out []string + for _, e := range entries { + out = append(out, fmt.Sprintf("%s@%s:%q", e.Manifest.Module, + strings.TrimSuffix(strings.TrimPrefix(e.Source.Repository, "http://forge.internal:20000/novox/"), ".git"), + e.Source.Path)) + } + return out +} From add807f034b12b47b734461a24fa903908150e20 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 8 Oct 2026 22:22:02 +0200 Subject: [PATCH 2/3] Say no cycle for a packages edge in a plan's last tier A packages edge orders nothing, so a module and what packages its source share a tier by rule; hasCycle counted the edge and the merge handler said "the last tier depends on itself" of plans with no cycle. Skip the kind as tiersOf does. The planner tests' cycle rows and property now pass. --- cmd/mesh-controller/planner_rules_test.go | 17 ++++++----------- cmd/mesh-controller/release_plan.go | 7 ++++++- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/cmd/mesh-controller/planner_rules_test.go b/cmd/mesh-controller/planner_rules_test.go index f6c0c5cf..1622f336 100644 --- a/cmd/mesh-controller/planner_rules_test.go +++ b/cmd/mesh-controller/planner_rules_test.go @@ -218,14 +218,9 @@ func TestAPlanIsWhatTheChangeTouchedAndWhatIsBuiltOnIt(t *testing.T) { if u := strings.Join(r.Unread, ","); u != c.unread { t.Errorf("%s: unread %q, wanted %q", c.what, u, c.unread) } - // BUG found by this test on main (8170fc5): hasCycle reads any edge between two modules of the last - // tier as a cycle — a packages edge too, which puts both there by rule — so the merge handler logs - // "the last tier depends on itself … built together, in no order" for a plan with no cycle (the - // packages rows and the mixed diamond fail here). ADR 0162: a cycle is one last tier, *and said*; - // one said where there is none is the rule broken. The expectation stays. + // A packages edge in the last tier is no cycle; hasCycle said one on main at 8170fc5. if hasCycle(r.Plan.Tiers, c.edges) != c.cycle { - t.Errorf("%s: a cycle said %v, wanted %v (%v) [BUG: hasCycle counts a packages edge]", - c.what, !c.cycle, c.cycle, r.Plan.Tiers) + t.Errorf("%s: a cycle said %v, wanted %v (%v)", c.what, !c.cycle, c.cycle, r.Plan.Tiers) } } } @@ -310,7 +305,7 @@ var sharedRepositoryEdges = []inventory.Edge{ // packages — never along built-by or worker-of; // - every stands-on, declared, built-by and worker-of edge with both ends in the plan has the module // depended on in an earlier tier; -// - no cycle is said (this one fails on main: see the BUG note below). +// - no cycle is said. // // Seeded, so a failure is replayed by its seed and case. func TestAPlanIsTheTouchedModulesAndWhatIsReachableAlongTheWideningEdges(t *testing.T) { @@ -413,10 +408,10 @@ func TestAPlanIsTheTouchedModulesAndWhatIsReachableAlongTheWideningEdges(t *test } } } - // BUG found by this property on main (8170fc5); see the table's cycle check. Said once, after the - // other invariants have run over every case, so it hides none of them. + // Said once, after the other invariants have run over every case, so it hides none of them (hasCycle + // counted a packages edge on main at 8170fc5: 19 of these 500 cases). if falseCycles > 0 { - t.Errorf("a cycle said of a graph with none in %d of 500 cases [BUG: hasCycle counts a packages edge]; "+ + t.Errorf("a cycle said of a graph with none in %d of 500 cases; "+ "the first:\n%s", falseCycles, firstFalseCycle) } } diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index c240d478..8688705b 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -157,7 +157,9 @@ func reachableFrom(moved []string, edges []inventory.Edge) []string { return out } -// hasCycle says whether the tiers' last tier holds modules that still depend on each other. +// hasCycle says whether the tiers' last tier holds modules that still depend on each other. A packages +// edge orders nothing (tiersOf), so a module and what packages its source share a tier by rule: that is +// no cycle, and saying one was is a false report in every such plan's log. func hasCycle(tiers [][]string, edges []inventory.Edge) bool { if len(tiers) == 0 { return false @@ -167,6 +169,9 @@ func hasCycle(tiers [][]string, edges []inventory.Edge) bool { last[m] = true } for _, e := range edges { + if e.Kind == inventory.EdgePackages { + continue + } if last[e.From] && last[e.To] { return true } From 758537dd4ea285ed2cffd9ba6555b1a9a9ae8b43 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 8 Oct 2026 22:22:24 +0200 Subject: [PATCH 3/3] Plan a controller merge in the worker-of order in the three-kinds test The test's fixture had no worker-of edge and expected the builder first, then the controller and the proxy together: the order before hq issue 206. Since then the build seat's holder follows the controller that defines its worker, and every controller merge plans controller, builder, proxy in three tiers. The fixture now carries the edge and the test that order. --- cmd/mesh-controller/planner_rules_test.go | 5 ++--- cmd/mesh-controller/release_plan_test.go | 15 ++++++++++----- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/cmd/mesh-controller/planner_rules_test.go b/cmd/mesh-controller/planner_rules_test.go index 1622f336..6b8dd23d 100644 --- a/cmd/mesh-controller/planner_rules_test.go +++ b/cmd/mesh-controller/planner_rules_test.go @@ -259,9 +259,8 @@ func TestASharedRepositoryMovesWhatPackagesItAsItDoesToday(t *testing.T) { paths []string want string }{ - // The live three-tier plan of 2026-10-08 (issue 338). It is also the worker-of order (issue 206) - // that TestAMergeIsPlannedInTiersAlongTheThreeKindsOfDependency's controller case predates: that - // fixture has no worker-of edge and expects the builder first, then the controller and the proxy. + // The live three-tier plan of 2026-10-08 (issue 338), in the worker-of order (issue 206) that + // TestAMergeIsPlannedInTiersAlongTheThreeKindsOfDependency's controller case holds too. {"a README of the controller's repository moves all three, in three tiers", "mesh-controller", []string{"README.md"}, "mesh-controller | build-agent | route-proxy"}, {"the controller's own code: the same", "mesh-controller", diff --git a/cmd/mesh-controller/release_plan_test.go b/cmd/mesh-controller/release_plan_test.go index 066c6023..118a495b 100644 --- a/cmd/mesh-controller/release_plan_test.go +++ b/cmd/mesh-controller/release_plan_test.go @@ -27,6 +27,8 @@ func TestAMergeIsPlannedInTiersAlongTheThreeKindsOfDependency(t *testing.T) { {From: "mesh-controller", To: "builder", Kind: inventory.EdgeBuiltBy}, {From: "mesh-tools", To: "builder", Kind: inventory.EdgeBuiltBy}, {From: "builder", To: "mesh-tools", Kind: inventory.EdgeStandsOn}, + // the build seat's holder follows the controller that defines its worker (hq issue 206) + {From: "builder", To: "mesh-controller", Kind: inventory.EdgeWorkerOf}, {From: "unrelated", To: "alpine", Kind: inventory.EdgeStandsOn}, } // The runtime image moved: everything on it, and what is built by what is on it. @@ -62,12 +64,15 @@ func TestAMergeIsPlannedInTiersAlongTheThreeKindsOfDependency(t *testing.T) { if len(small) != 3 { t.Fatalf("a controller merge rebuilds the controller and what packages it: %v", small) } - // The builder packages the controller's source (same tier by that edge) and the controller is - // built by the builder (next tier by that one): the builder first, then the controller and the - // proxy together — a code dependency in one tier, a runtime dependency across tiers. + // The builder and the proxy package the controller's source, which orders nothing. The builder holds + // the build seat, whose worker the controller defines, so it follows the controller (worker-of, + // novox/hq issue 206), and the controller's built-by edge to it yields: the controller is built by the + // build machine that is running. The proxy is built by the new builder: the controller, the builder, + // the proxy — the live plan of every controller merge. (This read "the builder, then the controller + // and the proxy together" before issue 206, and the fixture had no worker-of edge.) smallTiers := tiersOf(small, edges) - if len(smallTiers) != 2 || smallTiers[0][0] != "builder" || len(smallTiers[1]) != 2 { - t.Fatalf("the builder, then the controller and the proxy together: %v", smallTiers) + if got := tiered(smallTiers); got != "mesh-controller | builder | route-proxy" { + t.Fatalf("the controller, then the builder, then the proxy: %v", smallTiers) } // The builder alone moved: the builder, and nothing it builds. if only := reachableFrom([]string{"builder"}, edges); len(only) != 1 {