Order a branch's plans by its merges, and build the newest commit (hq issue 349)
A merge acted on late by the catch-up made its plan after the plan of the merge that followed it, superseded it by creation time, and folded its unbuilt modules into a plan at the older commit: on 2026-10-09 the forge's security fix (a082615b) was superseded by 8ff8197a. A plan now keeps its merge time (migration 0086), supersession follows it, and a merge older than an open plan of its branch is planned at that plan's commit, which contains it.
This commit is contained in:
jochen
@@ -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")
|
||||
}
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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;
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user