diff --git a/cmd/mesh-controller/issue349_test.go b/cmd/mesh-controller/issue349_test.go new file mode 100644 index 00000000..ff2d3ba7 --- /dev/null +++ b/cmd/mesh-controller/issue349_test.go @@ -0,0 +1,93 @@ +package main + +import ( + "strings" + "testing" + "time" + + "github.com/novox/mesh-controller/internal/catalogue" + "github.com/novox/mesh-controller/internal/inventory" + "github.com/novox/mesh-controller/internal/link" +) + +// novox/hq issue 349: on 2026-10-09 the plan of mesh-catalog at a082615b (the merge of a security fix to the +// forge's module) was "superseded at tier 0 by" the plan at 8ff8197a, the merge before it, which the +// catch-up acted on late; what the later plan had not built was folded into a plan at the commit before the +// fix. Plans of one branch are ordered by when the forge made their merges, and a merge older than an open +// plan of its branch is planned at that plan's commit. + +// TestTwoMergesActedOnInReverseOrderBuildTheNewerCommit replays it: the later merge acted on first, the +// earlier one second (the catch-up). One plan is left open, at the later commit, and it builds both. +func TestTwoMergesActedOnInReverseOrderBuildTheNewerCommit(t *testing.T) { + open := aCatalogueMesh(t) + ctx := t.Context() + asksWithPaths(t) + for _, m := range []string{"gitea", "notes"} { + if err := open.inventory.RegisterModule(ctx, catalogue.Manifest{Module: m, Version: "1"}, + inventory.Source{Repository: "novox/mesh-catalog", Seat: "git", Path: "modules/" + m, Ref: "main", + BuiltFrom: "c0", Head: "c0"}); err != nil { + t.Fatal(err) + } + } + at := time.Now().UTC().Add(-20 * time.Minute).Truncate(time.Second) + fix := link.SourceMoved{Owner: "novox", Repo: "mesh-catalog", Base: "main", Commit: "a082615bfix", + MergedAt: at.Add(2 * time.Minute).Format(time.RFC3339), + Paths: []string{"modules/gitea/module.json"}, ModuleDirs: []string{"modules/gitea"}, ModuleDirsSaid: true} + before := link.SourceMoved{Owner: "novox", Repo: "mesh-catalog", Base: "main", Commit: "8ff8197abefore", + MergedAt: at.Format(time.RFC3339), + Paths: []string{"modules/notes/module.json"}, ModuleDirs: []string{"modules/notes"}, ModuleDirsSaid: true} + for _, m := range []link.SourceMoved{fix, before} { + if err := (following{open: open}).SourceMoved(ctx, m); err != nil { + t.Fatal(err) + } + } + plans, err := open.inventory.OpenPlans(ctx) + if err != nil { + t.Fatal(err) + } + if len(plans) != 1 { + var said []string + for _, p := range plans { + said = append(said, p.ID+" "+p.Commit+" "+p.Note) + } + t.Fatalf("open plans: %s", strings.Join(said, "; ")) + } + p := plans[0] + if p.Commit != fix.Commit { + t.Fatalf("the open plan builds %s, not the newer commit %s", p.Commit, fix.Commit) + } + for _, m := range []string{"gitea", "notes"} { + if _, has := p.Modules[m]; !has { + t.Fatalf("the open plan at the newer commit does not build %s: %v", m, p.Modules) + } + } +} + +// Pure: the branch's order is the merges', where both plans know it; and a merge older than an open plan +// of its branch finds it. +func TestTheBranchOrderIsTheMerges(t *testing.T) { + t0 := time.Date(2026, 10, 9, 10, 0, 0, 0, time.UTC) + newerMerge := inventory.Plan{ID: "plan-1", Repository: "novox/mesh-catalog", Branch: "main", Commit: "a082615b", + Merged: t0.Add(time.Minute), Created: t0.Add(2 * time.Minute), State: inventory.PlanRolling} + olderMerge := inventory.Plan{ID: "plan-2", Repository: "novox/mesh-catalog", Branch: "main", Commit: "8ff8197a", + Merged: t0, Created: t0.Add(10 * time.Minute), State: inventory.PlanBuilding} + if earlierOnTheBranch(newerMerge, olderMerge) || !earlierOnTheBranch(olderMerge, newerMerge) { + t.Fatal("ordered by when the plans were made, not by when the merges were") + } + if _, closed := supersededBy(olderMerge, []inventory.Plan{newerMerge}, func(string) bool { return true }); len(closed) != 0 { + t.Fatalf("the plan of an older merge superseded a newer one: %s", closed[0].Note) + } + unknown := newerMerge + unknown.Merged = time.Time{} + if !earlierOnTheBranch(unknown, olderMerge) { + t.Fatal("without a merge time the plans' own order does not stand") + } + m := link.SourceMoved{Owner: "novox", Repo: "mesh-catalog", Base: "main", Commit: "8ff8197a", MergedAt: t0.Format(time.RFC3339)} + if p, ok := newestOnTheBranch(m, []inventory.Plan{newerMerge}); !ok || p.ID != "plan-1" { + t.Fatalf("the newer open plan of the branch was not found: %v %+v", ok, p) + } + m.Base = "release" + if _, ok := newestOnTheBranch(m, []inventory.Plan{newerMerge}); ok { + t.Fatal("another branch's plan was taken") + } +} diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index b2bd68e7..ae66a31b 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -188,11 +188,13 @@ func planOfMerge(m link.SourceMoved, moved []string, edges []inventory.Edge) inv for _, name := range set { modules[name] = &inventory.PlanModule{} } + merged, _ := time.Parse(time.RFC3339, m.MergedAt) return inventory.Plan{ ID: fmt.Sprintf("plan-%d", time.Now().UnixNano()), Repository: m.Owner + "/" + m.Repo, Branch: m.Base, Commit: m.Commit, + Merged: merged.UTC(), Created: time.Now().UTC(), State: inventory.PlanBuilding, Tiers: tiers, @@ -220,12 +222,20 @@ func planOfMerge(m link.SourceMoved, moved []string, edges []inventory.Edge) inv // // A plan with no branch recorded is from before branches were kept, and is superseded by the next // plan of its repository: what it had not built is folded in, so nothing is lost by it. +// +// **Newer is the branch's order, not the plans'** (novox/hq issue 349). A merge the bus did not hand +// over is acted on late, by the catch-up, so its plan is made after the plan of a merge that came after +// it — and that later-made plan of the earlier commit superseded the later merge's, and built what it +// folded in from the commit before the later merge: twice on 2026-10-09, once a security fix. So where +// both plans know when their merge was made, that decides; only where one does not do the plans' own +// times. A merge older than an open plan of its branch never reaches here at its own commit: it is +// planned at the newest open plan's commit, which contains it (newestOnTheBranch). func supersededBy(newer inventory.Plan, open []inventory.Plan, rollsOut func(string) bool) ([]string, []inventory.Plan) { folded := map[string]bool{} var closed []inventory.Plan for _, old := range open { if old.ID == newer.ID || !old.Open() || !strings.EqualFold(old.Repository, newer.Repository) || - (old.Branch != "" && old.Branch != newer.Branch) || !old.Created.Before(newer.Created) { + (old.Branch != "" && old.Branch != newer.Branch) || !earlierOnTheBranch(old, newer) { continue } var took []string @@ -254,6 +264,39 @@ func supersededBy(newer inventory.Plan, open []inventory.Plan, rollsOut func(str return out, closed } +// earlierOnTheBranch says plan a answers a merge made before b's: by when the forge made each merge where +// both are known, else by when each plan was made. A merge's own plan made again at the same commit +// (the same merge time) is ordered by when it was made. Pure. +func earlierOnTheBranch(a, b inventory.Plan) bool { + if !a.Merged.IsZero() && !b.Merged.IsZero() && !a.Merged.Equal(b.Merged) { + return a.Merged.Before(b.Merged) + } + return a.Created.Before(b.Created) +} + +// newestOnTheBranch is the open plan of m's repository and branch whose merge was made after m's, the +// newest of them; false when none was. Merges into one branch are made one on the other, so that plan's +// commit contains m's change: m is planned there, and the newest commit of the branch is what is built +// (novox/hq issue 349). Pure. +func newestOnTheBranch(m link.SourceMoved, open []inventory.Plan) (inventory.Plan, bool) { + merged, err := time.Parse(time.RFC3339, m.MergedAt) + if err != nil { + return inventory.Plan{}, false + } + var newest inventory.Plan + found := false + for _, p := range open { + if !p.Open() || p.Release != nil || p.Merged.IsZero() || !strings.EqualFold(p.Repository, m.Owner+"/"+m.Repo) || + p.Branch != m.Base || !p.Merged.After(merged) { + continue + } + if !found || p.Merged.After(newest.Merged) { + newest, found = p, true + } + } + return newest, found +} + // gates is what the next tier needs running from this one: a module of the tier that a later // tier is built by — the runtime dependency — and whose policy rolls it out, must be applied by // the machines running it before the next tier is asked. A base an image stands on need only be diff --git a/cmd/mesh-controller/upgrades.go b/cmd/mesh-controller/upgrades.go index 86b08215..5a639aa1 100644 --- a/cmd/mesh-controller/upgrades.go +++ b/cmd/mesh-controller/upgrades.go @@ -345,6 +345,20 @@ func (f following) SourceMoved(ctx context.Context, m link.SourceMoved) error { e.Manifest.Module, m.Owner, m.Repo, e.Source.Ref, m.Base) } } + // **A merge older than an open plan of its branch is planned at that plan's commit** (novox/hq issue + // 349): the catch-up acts on a merge the bus did not hand over after the merges that came after it, + // and planned at its own commit it built what the later merge had fixed from the commit before the fix. + // The later commit contains this merge's change, so what this merge moved is built from there, and the + // plan made here supersedes that one as any newer plan does. + if open, err := inv.OpenPlans(ctx); err == nil { + if newest, later := newestOnTheBranch(m, open); later { + fmt.Printf(" %s/%s %.8s was merged before %.8s, which %s is planning: what it moved is built "+ + "from %.8s, which contains it\n", m.Owner, m.Repo, m.Commit, newest.Commit, newest.ID, newest.Commit) + m.Commit, m.MergedAt = newest.Commit, newest.Merged.Format(time.RFC3339) + } + } else { + return notNow(err) + } touched, added, _ := touchedBy(from, entries, m) // **A module the merge deleted is not built** (novox/hq ADR 0236): its manifest is gone, so the build // seat finds nothing saying what it is, and the plan failed on it (`has no module.json at …`) with diff --git a/internal/inventory/migrations/0086-a-plan-keeps-when-its-merge-was-made.sql b/internal/inventory/migrations/0086-a-plan-keeps-when-its-merge-was-made.sql new file mode 100644 index 00000000..de593714 --- /dev/null +++ b/internal/inventory/migrations/0086-a-plan-keeps-when-its-merge-was-made.sql @@ -0,0 +1,10 @@ +-- A plan keeps when the forge made the merge it answers (novox/hq issue 349). +-- +-- A newer plan of a repository's branch supersedes the older open ones, and "newer" was read from when each +-- plan was made. A merge the bus did not hand over is acted on late, by the catch-up, so its plan is made +-- after the plan of a merge that came after it — and superseded it, folding its unbuilt modules into a plan +-- at the older commit. On 2026-10-09 a security fix to the forge's module would have been built from the +-- commit before it. Merges into one branch are made one after another, each on the one before, so the +-- forge's merge time is the branch's order. Null for a plan kept before this column, and for a plan no merge +-- made (a release); then the plans' own order stands, as before. +alter table release_plan add column merged_at timestamptz; diff --git a/internal/inventory/plans.go b/internal/inventory/plans.go index 00842dd4..348cf15d 100644 --- a/internal/inventory/plans.go +++ b/internal/inventory/plans.go @@ -19,8 +19,12 @@ type Plan struct { Repository string `json:"repository"` // Branch is the branch the merge went into (novox/hq issue 254): a newer plan supersedes the open // ones of the same repository and branch. Empty for a plan from before it was kept. - Branch string `json:"branch,omitempty"` - Commit string `json:"commit"` + Branch string `json:"branch,omitempty"` + Commit string `json:"commit"` + // Merged is when the forge made the merge this plan answers (novox/hq issue 349): the order of a + // branch's merges, which is not the order their plans were made in when one was acted on late. Zero + // for a release, and for a plan kept before it was. + Merged time.Time `json:"merged,omitzero"` Created time.Time `json:"created"` Updated time.Time `json:"updated"` State string `json:"state"` @@ -249,8 +253,8 @@ func (i *Inventory) SavePlan(ctx context.Context, p *Plan) error { var revision int64 err = tx.QueryRow(ctx, `insert into release_plan (id, repository, commit_hash, created, updated, state, tier, tiers, modules, note, - branch, tier_entered, revision, epoch, release, delivery) - values ($1, $2, $3, $4, now(), $5, $6, $7, $8, $9, $10, $11, 1, $13, $14, $15) + branch, tier_entered, revision, epoch, release, delivery, merged_at) + values ($1, $2, $3, $4, now(), $5, $6, $7, $8, $9, $10, $11, 1, $13, $14, $15, $16) on conflict (id) do update set updated = now(), state = excluded.state, tier = excluded.tier, tiers = excluded.tiers, modules = excluded.modules, note = excluded.note, branch = excluded.branch, tier_entered = excluded.tier_entered, revision = release_plan.revision + 1, epoch = excluded.epoch, @@ -258,7 +262,7 @@ func (i *Inventory) SavePlan(ctx context.Context, p *Plan) error { where release_plan.revision = $12 returning revision`, p.ID, p.Repository, p.Commit, p.Created, p.State, p.Tier, tiers, modules, p.Note, p.Branch, entered, - p.Revision, epoch, release, delivery).Scan(&revision) + p.Revision, epoch, release, delivery, mergedAt(p.Merged)).Scan(&revision) if errors.Is(err, pgx.ErrNoRows) { // The row is there and at another revision — moved since this was read, or there already // when this one is new: either way not this writer's to overwrite. (A plan saved before plans @@ -291,6 +295,14 @@ func short(commit string) string { return commit } +// mergedAt is a plan's merge time as the store keeps it: null when not known. +func mergedAt(t time.Time) *time.Time { + if t.IsZero() { + return nil + } + return &t +} + // OpenPlans is every plan still being worked, oldest first. func (i *Inventory) OpenPlans(ctx context.Context) ([]Plan, error) { return i.plans(ctx, `where state in ('building', 'rolling') order by created`) @@ -316,7 +328,7 @@ func (i *Inventory) PlanByID(ctx context.Context, id string) (Plan, error) { func (i *Inventory) plans(ctx context.Context, tail string) ([]Plan, error) { rows, err := i.store.Pool().Query(ctx, `select id, repository, commit_hash, created, updated, state, tier, tiers, modules, note, branch, - coalesce(tier_entered, created), revision, coalesce(epoch, 0), release, delivery + coalesce(tier_entered, created), revision, coalesce(epoch, 0), release, delivery, merged_at from release_plan `+tail) if err != nil { return nil, err @@ -327,11 +339,15 @@ func (i *Inventory) plans(ctx context.Context, tail string) ([]Plan, error) { var p Plan var tiers, modules, release, delivery []byte var epoch int64 + var merged *time.Time if err := rows.Scan(&p.ID, &p.Repository, &p.Commit, &p.Created, &p.Updated, &p.State, &p.Tier, &tiers, &modules, &p.Note, &p.Branch, &p.TierEntered, &p.Revision, &epoch, &release, - &delivery); err != nil { + &delivery, &merged); err != nil { return nil, err } + if merged != nil { + p.Merged = merged.UTC() + } if len(release) > 0 { if err := json.Unmarshal(release, &p.Release); err != nil { return nil, err