From 1560c498b67753dc22dfbbc3a8c34f835cc18633 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 10 Oct 2026 12:49:44 +0200 Subject: [PATCH 1/3] Ask the rollback test's failed build after the registered one, whenever it runs Its id named 2026-10-10 02:40 UTC; once the clock passed that, the registered build read as newer and the test failed on main. --- cmd/mesh-controller/build_source_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cmd/mesh-controller/build_source_test.go b/cmd/mesh-controller/build_source_test.go index f2589e1c..146ffcfe 100644 --- a/cmd/mesh-controller/build_source_test.go +++ b/cmd/mesh-controller/build_source_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "hash/fnv" "os" "os/exec" @@ -11,6 +12,7 @@ import ( "strings" "sync" "testing" + "time" "github.com/novox/mesh-controller/internal/inventory" "github.com/novox/mesh-controller/internal/link" @@ -208,7 +210,9 @@ func TestARollbackNeverPutsBackABuildFromAnotherRepository(t *testing.T) { if _, _, err := takeIn(ctx, inv, fork); !errors.Is(err, errNotItsSource) { t.Fatalf("the fork's build was taken in: %v", err) } - failed := onTrunk("build-1791600000000000000", "novox/mesh-catalog", "git", "modules/sudo", + // Asked after the registered build, whenever the test runs: an id naming a fixed moment read as older + // than the registered build once the clock passed it (2026-10-10 02:40 UTC), and the test failed on main. + failed := onTrunk(fmt.Sprintf("build-%d", time.Now().Add(time.Hour).UnixNano()), "novox/mesh-catalog", "git", "modules/sudo", map[string]any{"module": "sudo", "version": "2"}) failed.Commit = "badbadbad0123456" if _, _, err := takeIn(ctx, inv, failed); err != nil { From 887de9b5f2447be4afe2ba594e82e3ba059988d6 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 10 Oct 2026 12:49:44 +0200 Subject: [PATCH 2/3] Say why each module is in a plan: its build source's changed files, or why it is read whole (hq ADR 0267, issue 363) A what-if named the build seat's holder as packaging the controller's source, "rebuilt without their own source moving", while it was in the plan because an open plan had not built it yet. The what-if and the merge log now say, per module, which changed files of its build source moved it, or that it is read whole and why: no build source recorded, a newer build failed, or a plan has not built it yet. --- cmd/mesh-controller/planner_rules_test.go | 46 ++++++++++++ cmd/mesh-controller/release_plan.go | 25 ++++--- cmd/mesh-controller/upgrades.go | 86 ++++++++++++++++++++--- internal/inventory/builds.go | 14 +++- internal/inventory/builds_test.go | 4 +- 5 files changed, 152 insertions(+), 23 deletions(-) diff --git a/cmd/mesh-controller/planner_rules_test.go b/cmd/mesh-controller/planner_rules_test.go index f47cded2..3e07a6d1 100644 --- a/cmd/mesh-controller/planner_rules_test.go +++ b/cmd/mesh-controller/planner_rules_test.go @@ -626,3 +626,49 @@ func TestTheGatePlansFromTheBuildSourcesTheSnapshotCarries(t *testing.T) { t.Errorf("a snapshot without build sources: the gate planned %q for a README", got) } } + +// **A plan says why each module is in it** (novox/hq ADR 0267, issue 363): the files of its build source the +// merge changed, or why it is read whole — never that it packages a repository as though that moved it. +func TestAPlanSaysWhyEachModuleIsInIt(t *testing.T) { + const controllerRepo = "http://forge.internal:20000/novox/mesh-controller.git" + controller := fromRepo("mesh-controller", controllerRepo, "") + agent := fromRepo("build-agent", "http://forge.internal:20000/novox/mesh-catalog.git", "modules/build-agent") + gitea := fromRepo("gitea", "http://forge.internal:20000/novox/mesh-catalog.git", "modules/gitea") + built := time.Date(2026, 10, 10, 12, 26, 0, 0, time.UTC) + read := map[string][]inventory.ReadRepository{ + "mesh-controller": {{Own: true, Paths: []string{"module.json", "cmd/mesh-controller/", "internal/link/"}, Built: built}}, + "build-agent": { + {Repository: "novox/mesh-controller", Ref: "main", Paths: []string{"cmd/mesh-builder/", "internal/link/"}, Built: built}, + {Own: true, Paths: []string{"modules/build-agent/module.json"}, Built: built}, + }, + } + merge := func(repo string, paths ...string) link.SourceMoved { + return link.SourceMoved{Owner: "novox", Repo: repo, Base: "main", Paths: paths} + } + open := []inventory.Plan{{ID: "plan-1", State: inventory.PlanBuilding, Created: built.Add(time.Minute), + Modules: map[string]*inventory.PlanModule{"build-agent": {State: "asked"}}}} + for _, c := range []struct { + what string + e inventory.Entry + read map[string][]inventory.ReadRepository + m link.SourceMoved + says string + }{ + {"the controller's own closure", controller, read, merge("mesh-controller", "README.md", "internal/link/handacts.go"), + "its build source changed: 1 changed file(s) in it, e.g. internal/link/handacts.go"}, + {"the build seat's closure, through its context", agent, read, merge("mesh-controller", "internal/link/handacts.go"), + "its build source in novox/mesh-controller changed: 1 changed file(s) in it, e.g. internal/link/handacts.go"}, + {"an open plan has yet to build it", agent, planningView(read, open), merge("mesh-controller", "cmd/mesh-controller/main.go"), + "read whole: plan plan-1 has not built it yet, so every file of novox/mesh-controller, which its build context is, is its build source"}, + {"nothing recorded, built from its root", controller, nil, merge("mesh-controller", "README.md"), + "read whole: no build source recorded, so every file of its repository is its build source"}, + {"nothing recorded, in its directory", gitea, nil, merge("mesh-catalog", "modules/gitea/index.ts"), + "read whole: no build source recorded, so its directory is its build source; e.g. modules/gitea/index.ts"}, + {"files not all said", controller, read, link.SourceMoved{Owner: "novox", Repo: "mesh-controller", PathsTruncated: true, + Paths: []string{"x"}}, "read whole: the merge's changed files were not all said"}, + } { + if got := whyMoved(c.e, c.read[c.e.Manifest.Module], c.m); got != c.says { + t.Errorf("%s:\n said %q\n wanted %q", c.what, got, c.says) + } + } +} diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 05fc4773..e21b361c 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -1602,6 +1602,15 @@ func planWhatIf(ctx context.Context, inv *inventory.Inventory, repository string } p := planOfMerge(m, names, edges) fmt.Printf("a merge of %s would build %d module(s) in %d tier(s):\n", repository, len(p.Modules), len(p.Tiers)) + why := map[string]string{} + for _, e := range packaging { + why[e.Manifest.Module] = whyMoved(e, read[e.Manifest.Module], m) + } + if len(named) == 0 { + for _, e := range from { + why[e.Manifest.Module] = whyMoved(e, read[e.Manifest.Module], m) + } + } rolls := map[string]string{} for i, tier := range p.Tiers { fmt.Printf(" tier %d\n", i) @@ -1619,19 +1628,19 @@ func planWhatIf(ctx context.Context, inv *inventory.Inventory, repository string rolls[name] = how } fmt.Printf(" %-22s %s\n", name, how) + reason := why[name] + switch { + case reason == "" && len(named) > 0 && named[name]: + reason = "named" + case reason == "": + reason = "it stands on a module the merge moves" + } + fmt.Printf(" %-22s why: %s\n", "", reason) } } if hasCycle(p.Tiers, edges) { fmt.Println(" the last tier depends on itself and would be built together, in no order") } - if len(packaging) > 0 { - var also []string - for _, e := range packaging { - also = append(also, e.Manifest.Module) - } - fmt.Printf(" %s package source from %s, so they are rebuilt without their own source moving\n", - strings.Join(also, ", "), repository) - } return nil } diff --git a/cmd/mesh-controller/upgrades.go b/cmd/mesh-controller/upgrades.go index dc089f96..74261bc8 100644 --- a/cmd/mesh-controller/upgrades.go +++ b/cmd/mesh-controller/upgrades.go @@ -465,13 +465,13 @@ func (f following) SourceMoved(ctx context.Context, m link.SourceMoved) error { } fmt.Printf("%s/%s merged into %s (%.8s); plan %s, %d module(s) in %d tier(s)\n %s\n", m.Owner, m.Repo, m.Base, m.Commit, plan.ID, len(plan.Modules), len(plan.Tiers), strings.Join(tiers, "\n ")) - if len(packaging) > 0 { - var also []string - for _, e := range packaging { - also = append(also, e.Manifest.Module) - } - fmt.Printf(" %s package source from it, so they are rebuilt and their own source record "+ - "is left where it is\n", strings.Join(also, ", ")) + // Why each is in it (issue 363): the files of its build source the merge changed, or why it is read whole. + for _, e := range moved { + fmt.Printf(" %s: %s\n", e.Manifest.Module, whyMoved(e, read[e.Manifest.Module], m)) + } + for _, e := range packaging { + fmt.Printf(" %s reads %s/%s through its build context: its own source record is left where it is\n", + e.Manifest.Module, m.Owner, m.Repo) } if plan.Waiting() { plan.Note = waitingNote(plan) @@ -597,6 +597,63 @@ func lookedAtCommit(read []inventory.ReadRepository, commit string) bool { // historyMargin is how far a packaging module's last look is taken back before a merge is history for it. const historyMargin = time.Minute +// whyMoved is why a merge moves a module, as a plan says it (novox/hq ADR 0267, issue 363): the changed +// files in its build source, or why it is read whole. For a module built from the merged repository or one +// whose build context is that repository; a dependent is in a plan for what it stands on. +func whyMoved(e inventory.Entry, read []inventory.ReadRepository, m link.SourceMoved) string { + if len(m.Paths) == 0 || m.PathsTruncated { + return "read whole: the merge's changed files were not all said" + } + whole := "no build source recorded" + for _, r := range read { + if r.Own && r.Whole != "" { + whole = r.Whole + } + } + held := func(paths []string) []string { + var in []string + for _, p := range m.Paths { + if builder.SourceHolds(paths, p) { + in = append(in, p) + } + } + return in + } + changed := func(where string, in []string) string { + return fmt.Sprintf("its build source%s changed: %d changed file(s) in it, e.g. %s", where, len(in), in[0]) + } + if sameRepository(e.Source.Repository, m) { + if own := ownSource(read); own != nil { + if in := held(own); len(in) > 0 { + return changed("", in) + } + } + dir := strings.Trim(e.Source.Path, "/") + if dir == "" { + return "read whole: " + whole + ", so every file of its repository is its build source" + } + for _, p := range m.Paths { + if p == moduleManifestFile || inside(p, dir) { + return "read whole: " + whole + ", so its directory is its build source; e.g. " + p + } + } + return "read whole: " + whole + } + for _, r := range read { + if r.Own || !sameRepository(r.Repository, m) { + continue + } + if len(r.Paths) > 0 { + if in := held(r.Paths); len(in) > 0 { + return changed(" in "+m.Owner+"/"+m.Repo, in) + } + continue + } + return "read whole: " + whole + ", so every file of " + m.Owner + "/" + m.Repo + ", which its build context is, is its build source" + } + return "read whole: " + whole +} + // lookedOf is when a module packaging another repository was last looked at, as readForPlanning says. func lookedOf(read []inventory.ReadRepository) time.Time { var at time.Time @@ -757,8 +814,10 @@ func readsFile(e inventory.Entry, read []inventory.ReadRepository, p string) boo // or superseded. What such a module is built from is changing, or changed without a build to say so: a merge // that added an import to it, and a later one changing only what that import names, would otherwise move // nothing. Each is read whole, as before, until a build of it works again. -func staleIn(read map[string][]inventory.ReadRepository, plans []inventory.Plan) map[string]bool { - stale := map[string]bool{} +// +// Each is answered with the plan that overtook it, for saying why it is read whole. +func staleIn(read map[string][]inventory.ReadRepository, plans []inventory.Plan) map[string]string { + stale := map[string]string{} for name, rs := range read { var since time.Time for _, r := range rs { @@ -772,7 +831,7 @@ func staleIn(read map[string][]inventory.ReadRepository, plans []inventory.Plan) continue } if p.Open() || p.Created.After(since) { - stale[name] = true + stale[name] = p.ID } } } @@ -838,8 +897,9 @@ func planningView(read map[string][]inventory.ReadRepository, plans []inventory. } } var kept []inventory.ReadRepository + overtaken, isStale := stale[name] for _, r := range rs { - if stale[name] { + if isStale { if r.Own { continue } @@ -848,6 +908,10 @@ func planningView(read map[string][]inventory.ReadRepository, plans []inventory. r.Looked, r.LookedAt = looked, commits kept = append(kept, r) } + if isStale { + kept = append(kept, inventory.ReadRepository{Own: true, Whole: "plan " + overtaken + " has not built it yet", + Looked: looked, LookedAt: commits}) + } out[name] = kept } return out diff --git a/internal/inventory/builds.go b/internal/inventory/builds.go index 73f6020a..c85d9f42 100644 --- a/internal/inventory/builds.go +++ b/internal/inventory/builds.go @@ -98,6 +98,9 @@ type ReadRepository struct { // LookedAt are the merge commits a plan that built the module, or is building it, answered: a merge // of one of them is history for the module whatever the clocks say. Never stored. LookedAt []string `json:"-"` + // Whole is why the module is read whole though a build source may have been recorded — a newer build + // failed, or a plan has not built it yet — said on an Own entry with no Paths. Never stored. + Whole string `json:"-"` } // BuildSource is the build source a build read in one repository (novox/hq ADR 0267): Repository and Ref @@ -408,10 +411,15 @@ func (i *Inventory) ReadRepositories(ctx context.Context) (map[string][]ReadRepo sources = nil } } - if n, known := newest[module]; known && n.failed && n.at.After(built) { - sources = nil + whole := "" + if n, known := newest[module]; known && n.failed && n.at.After(built) && len(sources) > 0 { + sources, whole = nil, "a newer build of it failed" } - if of = WithBuildSources(of, sources); len(of) > 0 { + of = WithBuildSources(of, sources) + if whole != "" { + of = append(of, ReadRepository{Own: true, Whole: whole}) + } + if len(of) > 0 { for k := range of { of[k].Built, of[k].Looked = built, newest[module].at } diff --git a/internal/inventory/builds_test.go b/internal/inventory/builds_test.go index f2e0fe9b..8608ff0b 100644 --- a/internal/inventory/builds_test.go +++ b/internal/inventory/builds_test.go @@ -243,7 +243,9 @@ func TestAFailedNewerBuildLeavesTheBuildSourceStale(t *testing.T) { if err != nil { t.Fatal(err) } - if got := read["route-proxy"]; len(got) != 1 || got[0].Paths != nil || got[0].Own || !got[0].Looked.After(got[0].Built) { + got := read["route-proxy"] + if len(got) != 2 || got[0].Paths != nil || got[0].Own || !got[0].Looked.After(got[0].Built) || + !got[1].Own || got[1].Paths != nil || got[1].Whole != "a newer build of it failed" { t.Fatalf("after a newer failed build the proxy reads as %+v", got) } } From d0161fac5262731768e66729134e2b484debe0ae Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 10 Oct 2026 12:56:22 +0200 Subject: [PATCH 3/3] Say a deleted module's reason, read a context's reason at the merged branch, and keep the snapshot's bytes (hq ADR 0267, review) --- cmd/mesh-controller/facts.go | 4 +++- cmd/mesh-controller/planner_rules_test.go | 6 ++++++ cmd/mesh-controller/release_plan.go | 7 +++++++ cmd/mesh-controller/upgrades.go | 4 ++-- 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/cmd/mesh-controller/facts.go b/cmd/mesh-controller/facts.go index 95a99bc8..d24cc913 100644 --- a/cmd/mesh-controller/facts.go +++ b/cmd/mesh-controller/facts.go @@ -414,7 +414,9 @@ func gatherFacts(ctx context.Context, open *stores, busVersion string) (snapshot // The module's own build source is said apart: a gate that predates it would read an own // entry among Reads as a context of its own repository. if r.Own { - mod.Sources = append(mod.Sources, snapshot.BuildSource{Own: true, Paths: r.Paths}) + if len(r.Paths) > 0 { + mod.Sources = append(mod.Sources, snapshot.BuildSource{Own: true, Paths: r.Paths}) + } continue } mod.Reads = append(mod.Reads, snapshot.RepositoryName(r.Repository)) diff --git a/cmd/mesh-controller/planner_rules_test.go b/cmd/mesh-controller/planner_rules_test.go index 3e07a6d1..69ed505f 100644 --- a/cmd/mesh-controller/planner_rules_test.go +++ b/cmd/mesh-controller/planner_rules_test.go @@ -664,6 +664,12 @@ func TestAPlanSaysWhyEachModuleIsInIt(t *testing.T) { "read whole: no build source recorded, so every file of its repository is its build source"}, {"nothing recorded, in its directory", gitea, nil, merge("mesh-catalog", "modules/gitea/index.ts"), "read whole: no build source recorded, so its directory is its build source; e.g. modules/gitea/index.ts"}, + {"a root manifest is not in a module's directory", gitea, nil, merge("mesh-catalog", "module.json", "modules/gitea/x.ts"), + "read whole: no build source recorded, so its directory is its build source; e.g. modules/gitea/x.ts"}, + {"a context on another branch says nothing of this one", agent, map[string][]inventory.ReadRepository{"build-agent": { + {Repository: "novox/mesh-controller", Ref: "release", Paths: []string{"internal/link/"}}, + {Repository: "novox/mesh-controller", Ref: "main"}}}, merge("mesh-controller", "internal/link/handacts.go"), + "read whole: no build source recorded, so every file of novox/mesh-controller, which its build context is, is its build source"}, {"files not all said", controller, read, link.SourceMoved{Owner: "novox", Repo: "mesh-controller", PathsTruncated: true, Paths: []string{"x"}}, "read whole: the merge's changed files were not all said"}, } { diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index e21b361c..70570eea 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -1567,6 +1567,7 @@ func planWhatIf(ctx context.Context, inv *inventory.Inventory, repository string return err } var from, packaging []inventory.Entry + deleted := map[string]bool{} named := map[string]bool{} for _, name := range modules { named[name] = true @@ -1576,6 +1577,9 @@ func planWhatIf(ctx context.Context, inv *inventory.Inventory, repository string // request's check ask it (novox/hq ADR 0238). r := reachOfMerge(m, entries, read, nil) from, packaging = append(append([]inventory.Entry{}, r.Touched...), r.Deleted...), r.Packaging + for _, e := range r.Deleted { + deleted[e.Manifest.Module] = true + } } else { for _, e := range entries { switch { @@ -1609,6 +1613,9 @@ func planWhatIf(ctx context.Context, inv *inventory.Inventory, repository string if len(named) == 0 { for _, e := range from { why[e.Manifest.Module] = whyMoved(e, read[e.Manifest.Module], m) + if deleted[e.Manifest.Module] { + why[e.Manifest.Module] = "its manifest is removed: deleted at its source, forgotten where nothing holds it, not built" + } } } rolls := map[string]string{} diff --git a/cmd/mesh-controller/upgrades.go b/cmd/mesh-controller/upgrades.go index 74261bc8..8ad71f1a 100644 --- a/cmd/mesh-controller/upgrades.go +++ b/cmd/mesh-controller/upgrades.go @@ -633,14 +633,14 @@ func whyMoved(e inventory.Entry, read []inventory.ReadRepository, m link.SourceM return "read whole: " + whole + ", so every file of its repository is its build source" } for _, p := range m.Paths { - if p == moduleManifestFile || inside(p, dir) { + if inside(p, dir) { return "read whole: " + whole + ", so its directory is its build source; e.g. " + p } } return "read whole: " + whole } for _, r := range read { - if r.Own || !sameRepository(r.Repository, m) { + if r.Own || !sameRepository(r.Repository, m) || (r.Ref != "" && r.Ref != m.Base) { continue } if len(r.Paths) > 0 {