diff --git a/cmd/mesh-controller/checks.go b/cmd/mesh-controller/checks.go index 478fceab..2fdfbddd 100644 --- a/cmd/mesh-controller/checks.go +++ b/cmd/mesh-controller/checks.go @@ -143,7 +143,7 @@ func (f following) PullUpdated(ctx context.Context, p link.PullUpdated) error { if err != nil { return err } - read, err := inv.ReadRepositories(ctx) + read, err := readForPlanning(ctx, inv) if err != nil { return err } diff --git a/cmd/mesh-controller/delivery.go b/cmd/mesh-controller/delivery.go index 4bffcb9c..b9bd0161 100644 --- a/cmd/mesh-controller/delivery.go +++ b/cmd/mesh-controller/delivery.go @@ -604,7 +604,7 @@ func theGraph(ctx context.Context, inv *inventory.Inventory) ([]inventory.Entry, if err != nil { return nil, nil, nil, err } - read, err := inv.ReadRepositories(ctx) + read, err := readForPlanning(ctx, inv) if err != nil { return nil, nil, nil, err } diff --git a/cmd/mesh-controller/facts.go b/cmd/mesh-controller/facts.go index 2d0cda96..95a99bc8 100644 --- a/cmd/mesh-controller/facts.go +++ b/cmd/mesh-controller/facts.go @@ -205,7 +205,7 @@ func gatherFacts(ctx context.Context, open *stores, busVersion string) (snapshot if err != nil { return snapshot.Facts{}, err } - read, err := inv.ReadRepositories(ctx) + read, err := readForPlanning(ctx, inv) if err != nil { return snapshot.Facts{}, err } diff --git a/cmd/mesh-controller/missed_merges.go b/cmd/mesh-controller/missed_merges.go index fb43871f..6428a2a7 100644 --- a/cmd/mesh-controller/missed_merges.go +++ b/cmd/mesh-controller/missed_merges.go @@ -48,7 +48,7 @@ func catchingUpOnMerges(ctx context.Context, open *stores, announced merges) { if err != nil { return nil, nil, err } - read, err := readForAMerge(ctx, open.inventory) + read, err := readForPlanning(ctx, open.inventory) return entries, read, err } failing := "" diff --git a/cmd/mesh-controller/planner_rules_test.go b/cmd/mesh-controller/planner_rules_test.go index 1aab2260..8f591813 100644 --- a/cmd/mesh-controller/planner_rules_test.go +++ b/cmd/mesh-controller/planner_rules_test.go @@ -7,6 +7,7 @@ import ( "sort" "strings" "testing" + "time" snapshot "github.com/novox/mesh-controller/internal/facts" "github.com/novox/mesh-controller/internal/inventory" @@ -335,41 +336,77 @@ func TestASharedRepositoryMovesOnlyWhatItsBuildSourceHolds(t *testing.T) { } } -// **A module an open plan has yet to build is read whole** (novox/hq ADR 0267): its build source is about to -// change. A merge that added an import to the route proxy is planned; before that build lands, a merge -// changing only the package newly imported must still move the proxy — the build source its last build -// said does not hold it yet. -func TestAModuleAPlanHasYetToBuildIsReadWhole(t *testing.T) { +// **A module whose recorded build source a plan has overtaken is read whole** (novox/hq ADR 0267): a merge +// that added an import to the route proxy is planned; before a build of it works — still building, failed, +// or its plan closed before reaching it — a merge changing only the newly imported package must still move +// the proxy, since the build source its last build said does not hold that package. +func TestAModuleAPlanOvertookIsReadWhole(t *testing.T) { + built := time.Date(2026, 10, 10, 1, 0, 0, 0, time.UTC) read := map[string][]inventory.ReadRepository{ "route-proxy": { - {Repository: "novox/mesh-controller", Ref: "main", Paths: []string{"examples/route-proxy/", "go.mod"}}, - {Own: true, Paths: []string{"modules/route-proxy/module.json"}}, + {Repository: "novox/mesh-controller", Ref: "main", Paths: []string{"examples/route-proxy/", "go.mod"}, Built: built}, + {Own: true, Paths: []string{"modules/route-proxy/module.json"}, Built: built}, }, - "mesh-controller": {{Own: true, Paths: []string{"module.json", "cmd/mesh-controller/"}}}, - } - open := []inventory.Plan{{State: inventory.PlanBuilding, Modules: map[string]*inventory.PlanModule{ - "route-proxy": {State: "building"}, "gitea": {State: "built"}}}} - pending := pendingIn(open) - if !pending["route-proxy"] || pending["gitea"] { - t.Fatalf("pending %v", pending) + "mesh-controller": {{Own: true, Paths: []string{"module.json", "cmd/mesh-controller/"}, Built: built}}, } m := link.SourceMoved{Owner: "novox", Repo: "mesh-controller", Base: "main", Paths: []string{"internal/newly/imported.go"}} if readsFrom(read["route-proxy"], m) { t.Fatal("the said build source holds the new package: the fixture is wrong") } - whole := unnarrowed(read, pending) - if !readsFrom(whole["route-proxy"], m) { - t.Error("a module a plan has yet to build was read through the build source it is about to replace") + proxy := func(state string) map[string]*inventory.PlanModule { + return map[string]*inventory.PlanModule{"route-proxy": {State: state}, "gitea": {State: "built"}} } - if ownSource(whole["route-proxy"]) != nil { - t.Error("a pending module kept its own build source") + for _, c := range []struct { + what string + plans []inventory.Plan + whole bool + }{ + {"no plan", nil, false}, + {"a plan still building it", []inventory.Plan{{State: inventory.PlanBuilding, Created: built.Add(-time.Hour), + Modules: proxy("building")}}, true}, + {"a plan made after its build that failed before building it", []inventory.Plan{{State: "failed", + Created: built.Add(time.Minute), Modules: proxy("waiting")}}, true}, + {"a plan made after its build that built it", []inventory.Plan{{State: inventory.PlanDone, + Created: built.Add(time.Minute), Modules: proxy("built")}}, false}, + {"a plan closed before its build", []inventory.Plan{{State: "failed", Created: built.Add(-time.Hour), + Modules: proxy("waiting")}}, false}, + } { + view := planningView(read, c.plans) + if readsFrom(view["route-proxy"], m) != c.whole { + t.Errorf("%s: read whole %v, wanted %v", c.what, !c.whole, c.whole) + } + if (ownSource(view["route-proxy"]) == nil) != c.whole { + t.Errorf("%s: its own build source kept %v", c.what, ownSource(view["route-proxy"]) != nil) + } + if ownSource(view["mesh-controller"]) == nil { + t.Errorf("%s: a module no plan holds lost its build source", c.what) + } } - if ownSource(whole["mesh-controller"]) == nil { - t.Error("a module no plan is building lost its build source") +} + +// **A missed merge that moves only a module packaging the repository is acted on** (novox/hq ADR 0267, +// issue 266): the catch-up asks wouldMove, which counts it; once a plan or build of it is made after the +// merge, the merge is history for it, and the catch-up leaves it. +func TestAMissedMergeMovingOnlyAPackagingModuleIsActedOnOnce(t *testing.T) { + merged := time.Date(2026, 10, 10, 1, 0, 0, 0, time.UTC) + entries := []inventory.Entry{ + fromRepo("mesh-controller", "http://forge.internal:20000/novox/mesh-controller.git", ""), + fromRepo("route-proxy", "http://forge.internal:20000/novox/mesh-catalog.git", "modules/route-proxy"), } - // A closed plan holds nothing back. - if len(pendingIn([]inventory.Plan{{State: inventory.PlanDone, Modules: open[0].Modules}})) != 0 { - t.Error("a plan that is done held a module back") + read := map[string][]inventory.ReadRepository{ + "mesh-controller": {{Own: true, Paths: []string{"module.json", "cmd/mesh-controller/"}, Built: merged.Add(-time.Hour)}}, + "route-proxy": {{Repository: "novox/mesh-controller", Ref: "main", Paths: []string{"examples/route-proxy/"}, + Built: merged.Add(-time.Hour), Looked: merged.Add(-time.Hour)}}, + } + m := link.SourceMoved{Owner: "novox", Repo: "mesh-controller", Base: "main", Commit: "c1", + MergedAt: merged.Format(time.RFC3339), Paths: []string{"examples/route-proxy/main.go"}} + if got := wouldMove(m, entries, planningView(read, nil)); len(got) != 1 || got[0].Manifest.Module != "route-proxy" { + t.Fatalf("a missed merge of the proxy's program would move %v", got) + } + acted := []inventory.Plan{{State: inventory.PlanBuilding, Created: merged.Add(time.Minute), + Modules: map[string]*inventory.PlanModule{"route-proxy": {State: "building"}}}} + if got := wouldMove(m, entries, planningView(read, acted)); len(got) != 0 { + t.Fatalf("a merge acted on for the proxy would move %v again", got) } } diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 039d5a22..05fc4773 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -1562,7 +1562,7 @@ func planWhatIf(ctx context.Context, inv *inventory.Inventory, repository string if err != nil { return err } - read, err := inv.ReadRepositories(ctx) + read, err := readForPlanning(ctx, inv) if err != nil { return err } diff --git a/cmd/mesh-controller/upgrades.go b/cmd/mesh-controller/upgrades.go index 3288559e..9a41fe48 100644 --- a/cmd/mesh-controller/upgrades.go +++ b/cmd/mesh-controller/upgrades.go @@ -314,7 +314,7 @@ func (f following) SourceMoved(ctx context.Context, m link.SourceMoved) error { if err != nil { return notNow(err) } - read, err := readForAMerge(ctx, inv) + read, err := readForPlanning(ctx, inv) if err != nil { return notNow(err) } @@ -347,12 +347,6 @@ func (f following) SourceMoved(ctx context.Context, m link.SourceMoved) error { m.Owner, m.Repo, m.Base, m.Commit) return nil } - // The same judgement for the packaging kind, against the newest look at that repository by - // anything built from it: they keep no record of it themselves, and a replayed old merge should - // not rebuild them either. - if isHistory(m.MergedAt, lastLookAt(entries, m)) { - packaging = nil - } // Said, never silent (novox/hq 04-ISSUES/215): a module built from this repository that follows // another branch is not part of this merge, and whoever is waiting for its change should read why. for _, e := range entries { @@ -571,25 +565,43 @@ func mergeCandidates(m link.SourceMoved, entries []inventory.Entry, } from = append(from, e) case readsFrom(read[e.Manifest.Module], m): + // **A merge older than the module's last look is history for it** (novox/hq ADR 0267): a + // build or plan of it after the merge already read the repository with the merge in it. Per + // module, since a merge that moved only the module built from the repository says nothing + // about the ones packaging it. + if isHistory(m.MergedAt, lookedOf(read[e.Manifest.Module])) { + continue + } packaging = append(packaging, e) } } return from, packaging, already } -// wouldMove is the modules built from the merged repository that acting on this merge would mark as -// moved and rebuild — SourceMoved's judgement, made without acting (novox/hq issue 266). Empty for a -// merge already acted on: acting marks each of them as looked at, so the merge then reads as history. +// 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 + for _, r := range read { + if r.Looked.After(at) { + at = r.Looked + } + } + return at +} + +// wouldMove is the modules acting on this merge would move and rebuild — SourceMoved's judgement, made +// without acting (novox/hq issue 266). Empty for a merge already acted on: acting marks each module built +// from the repository as looked at, so the merge then reads as history for it. // -// **Only the modules built from it, never the ones that merely package source from it.** Acting -// records nothing about those, so a merge acted on would go on reading as unacted for them, and be -// acted on again on every look. A merge that moves both is caught by the first kind, and acting on it -// rebuilds the second as well. +// **The ones packaging source from it too** (novox/hq ADR 0267): with a module moved only by the files of +// its build source, a merge can move a packaging module and nothing built from the repository, and a missed +// one of those was never acted on. A packaging module's look is its newest build or plan (lookedAt), so a +// merge acted on for it reads as history once its plan is made. func wouldMove(m link.SourceMoved, entries []inventory.Entry, read map[string][]inventory.ReadRepository) []inventory.Entry { - from, _, _ := mergeCandidates(m, entries, read) + from, packaging, _ := mergeCandidates(m, entries, read) touched, _ := splitDeleted(whatTheMergeTouched(from, entries, m, read), m) - return touched + return append(touched, packaging...) } // splitDeleted parts the modules a merge touched into those it changed and those whose manifest it @@ -721,75 +733,92 @@ func readsFile(e inventory.Entry, read []inventory.ReadRepository, p string) boo return strings.Trim(e.Source.Path, "/") == "" || inside(p, e.Source.Path) } -// pendingIn is the modules an open plan has yet to build (novox/hq ADR 0267): what such a module was built -// from is about to change, so the build source its last build said is not the one a merge now meets — an -// earlier merge that added an import to it would otherwise hide a change to what that import names. Each -// is read whole, as before, and planned again with the merge; the newer plan supersedes the older. -func pendingIn(plans []inventory.Plan) map[string]bool { - pending := map[string]bool{} - for _, p := range plans { - if !p.Open() { - continue +// staleIn is the modules whose recorded build source a plan has overtaken (novox/hq ADR 0267): a plan still +// working that has yet to build one, or a plan made after that build which never built it — failed, stopped +// 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{} + for name, rs := range read { + var since time.Time + for _, r := range rs { + if r.Built.After(since) { + since = r.Built + } } - for name, s := range p.Modules { - if s == nil || (s.State != "built" && s.State != planDeleted) { - pending[name] = true + for _, p := range plans { + s, in := p.Modules[name] + if !in || (s != nil && (s.State == "built" || s.State == planDeleted)) { + continue + } + if p.Open() || p.Created.After(since) { + stale[name] = true } } } - return pending + return stale } -// readForAMerge is what each module's build read, as a merge acting now maps its files onto: the build -// sources the newest trunk builds said, but for the modules an open plan has yet to build (pendingIn). -func readForAMerge(ctx context.Context, inv *inventory.Inventory) (map[string][]inventory.ReadRepository, error) { +// lookedAt is when a merge was last acted on for a module that packages another repository's source: its +// newest build, or the newest plan that held it, whichever is later. A build asked after a merge clones that +// repository with the merge in it, so an older merge is history for it. +func lookedAt(name string, read []inventory.ReadRepository, plans []inventory.Plan) time.Time { + var at time.Time + for _, r := range read { + if r.Looked.After(at) { + at = r.Looked + } + } + for _, p := range plans { + if _, in := p.Modules[name]; in && p.Created.After(at) { + at = p.Created + } + } + return at +} + +// readForPlanning is what each module's build read, as the planner maps a change onto it — for a merge +// acting now, the merge gate, a pull request's check, a delivery's order and the what-if alike, so planning +// and gating cannot disagree (novox/hq ADR 0238): the build sources the newest trunk builds said, but for +// the modules a plan has overtaken (staleIn), and with when each was last looked at (lookedAt). +func readForPlanning(ctx context.Context, inv *inventory.Inventory) (map[string][]inventory.ReadRepository, error) { read, err := inv.ReadRepositories(ctx) if err != nil { return nil, err } - open, err := inv.OpenPlans(ctx) + plans, err := inv.RecentPlans(ctx, planLookBack) if err != nil { return nil, err } - return unnarrowed(read, pendingIn(open)), nil + return planningView(read, plans), nil } -// unnarrowed is what modules read, with the build sources of the pending ones set aside: each of them is -// read whole. -func unnarrowed(read map[string][]inventory.ReadRepository, pending map[string]bool) map[string][]inventory.ReadRepository { - if len(pending) == 0 { - return read - } +// planLookBack is how many recent plans judge whether a recorded build source is overtaken. +const planLookBack = 500 + +// planningView is readForPlanning over what was read, so a test can hand it records. +func planningView(read map[string][]inventory.ReadRepository, plans []inventory.Plan) map[string][]inventory.ReadRepository { + stale := staleIn(read, plans) out := make(map[string][]inventory.ReadRepository, len(read)) for name, rs := range read { - if !pending[name] { - out[name] = rs - continue - } - var whole []inventory.ReadRepository + looked := lookedAt(name, rs, plans) + var kept []inventory.ReadRepository for _, r := range rs { - if r.Own { - continue + if stale[name] { + if r.Own { + continue + } + r.Paths = nil } - r.Paths = nil - whole = append(whole, r) + r.Looked = looked + kept = append(kept, r) } - out[name] = whole + out[name] = kept } return out } -// lastLookAt is the most recent look at this repository by anything built from it. -func lastLookAt(entries []inventory.Entry, m link.SourceMoved) time.Time { - var newest time.Time - for _, e := range entries { - if sameRepository(e.Source.Repository, m) && e.Source.Seen.After(newest) { - newest = e.Source.Seen - } - } - return newest -} - // whatTheMergeTouched narrows the modules built from a repository to the ones the merge changed: **a // changed file touches exactly the modules whose build reads it** (novox/hq issue 280, ADR 0238). It is // touchedBy's first answer; touchedBy is the one place the mesh maps a changed file onto its modules. diff --git a/internal/inventory/builds.go b/internal/inventory/builds.go index 178c32d8..0550f195 100644 --- a/internal/inventory/builds.go +++ b/internal/inventory/builds.go @@ -91,6 +91,10 @@ type ReadRepository struct { // Own is the module's own repository, whose Paths narrow what its own directory — or, for a module // built from its repository's root, the whole repository — would otherwise be. Own bool `json:"own,omitempty"` + // Built is when the build these were read from was asked, and Looked when the module's newest build of + // any outcome was: what the planner judges a build source's age, and a merge's news, by. Never stored. + Built time.Time `json:"-"` + Looked time.Time `json:"-"` } // BuildSource is the build source a build read in one repository (novox/hq ADR 0267): Repository and Ref @@ -338,8 +342,41 @@ func (i *Inventory) BuiltAgainst(ctx context.Context) (map[string][]string, erro // none — off the trunk, from a builder that predates it, or of a source not known — leaves its module read // as before: every file of a context, and its own directory or root. func (i *Inventory) ReadRepositories(ctx context.Context) (map[string][]ReadRepository, error) { + // The newest build of each module whatever its outcome: a failed one newer than the newest that + // worked leaves that one's build source stale — the merge it was asked for may have changed the closure + // (novox/hq ADR 0267), so its module is read whole until a build works again. + newest := map[string]struct { + at time.Time + failed bool + }{} + tried, err := i.store.Pool().Query(ctx, + `select distinct on (module) module, coalesce(asked, at), failed <> '' + from build + where module is not null and module <> '' + order by module, `+newestRequestFirst) + if err != nil { + return nil, err + } + for tried.Next() { + var module string + var at time.Time + var failed bool + if err := tried.Scan(&module, &at, &failed); err != nil { + tried.Close() + return nil, err + } + newest[module] = struct { + at time.Time + failed bool + }{at, failed} + } + tried.Close() + if err := tried.Err(); err != nil { + return nil, err + } + rows, err := i.store.Pool().Query(ctx, - `select distinct on (module) module, built_contexts, build_sources + `select distinct on (module) module, built_contexts, build_sources, coalesce(asked, at) from build where module is not null and module <> '' and failed = '' order by module, `+newestRequestFirst) @@ -352,7 +389,8 @@ func (i *Inventory) ReadRepositories(ctx context.Context) (map[string][]ReadRepo for rows.Next() { var module string var raw, rawSources []byte - if err := rows.Scan(&module, &raw, &rawSources); err != nil { + var built time.Time + if err := rows.Scan(&module, &raw, &rawSources, &built); err != nil { return nil, err } var of []ReadRepository @@ -367,7 +405,13 @@ 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 + } if of = WithBuildSources(of, sources); len(of) > 0 { + for k := range of { + of[k].Built, of[k].Looked = built, newest[module].at + } read[module] = of } } diff --git a/internal/inventory/builds_test.go b/internal/inventory/builds_test.go index fda48b9e..f2e0fe9b 100644 --- a/internal/inventory/builds_test.go +++ b/internal/inventory/builds_test.go @@ -5,6 +5,7 @@ import ( "reflect" "strings" "testing" + "time" ) // A build result was answered to whoever asked and kept nowhere, so "when did this last build", @@ -200,6 +201,12 @@ func TestABuildsBuildSourceComesBackOverWhatItRead(t *testing.T) { {Repository: "novox/other"}, {Paths: []string{"modules/route-proxy/Dockerfile", "modules/route-proxy/module.json"}, Own: true}, } + for k := range read["route-proxy"] { + if read["route-proxy"][k].Built.IsZero() { + t.Errorf("no build time on %+v", read["route-proxy"][k]) + } + read["route-proxy"][k].Built, read["route-proxy"][k].Looked = time.Time{}, time.Time{} + } if !reflect.DeepEqual(read["route-proxy"], want) { t.Fatalf("read back %+v\nwanted %+v", read["route-proxy"], want) } @@ -214,3 +221,29 @@ func TestABuildsBuildSourceComesBackOverWhatItRead(t *testing.T) { t.Fatalf("the build source was stored among what the build read: %s", stored) } } + +// A build newer than the newest that worked, and failed, leaves that one's build source stale: its module is +// read whole until a build works again (novox/hq ADR 0267). +func TestAFailedNewerBuildLeavesTheBuildSourceStale(t *testing.T) { + inv := fresh(t) + ctx := context.Background() + worked := aBuild("worked", "route-proxy", "") + worked.Asked = time.Now().Add(-time.Hour) + worked.Read = []ReadRepository{{Repository: "novox/mesh-controller", Ref: "main"}} + worked.Sources = []BuildSource{{Paths: []string{"modules/route-proxy/module.json"}}, + {Repository: "novox/mesh-controller", Ref: "main", Paths: []string{"examples/route-proxy/"}}} + failed := aBuild("failed", "route-proxy", "compile error") + failed.Asked = time.Now() + for _, b := range []Build{worked, failed} { + if err := inv.RecordBuild(ctx, b); err != nil { + t.Fatal(err) + } + } + read, err := inv.ReadRepositories(ctx) + 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) { + t.Fatalf("after a newer failed build the proxy reads as %+v", got) + } +}