Read a module whole while a plan has overtaken its build source, and plan every view alike (hq ADR 0267, review)

A failed build, or a plan closed before reaching a module, left the
closure its last good build said, and a fix-forward to a newly imported
package would have moved nothing. A missed merge moving only a module
that packages the repository was never caught up, and an older merge
read as history for it through a look that was not its own. The gate,
a pull request's check, the what-if and a delivery's order now read the
same view the merge handler does.
This commit is contained in:
jochen
2026-10-10 03:20:36 +02:00
parent 6c616838a5
commit 150f038ff8
9 changed files with 235 additions and 92 deletions
+1 -1
View File
@@ -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
}
+1 -1
View File
@@ -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
}
+1 -1
View File
@@ -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
}
+1 -1
View File
@@ -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 := ""
+61 -24
View File
@@ -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)
}
}
+1 -1
View File
@@ -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
}
+90 -61
View File
@@ -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.
+46 -2
View File
@@ -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
}
}
+33
View File
@@ -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)
}
}