From 0e0e143055078778549e72d2426028a4e50c5bf4 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 10 Oct 2026 13:48:31 +0200 Subject: [PATCH] Keep a merge a cut made history, and build a batch's walk at the branch (hq ADR 0276 review) --- cmd/mesh-controller/batches.go | 26 +++++++++++++-- cmd/mesh-controller/batches_test.go | 48 ++++++++++++++++++++++++++-- cmd/mesh-controller/missed_merges.go | 27 ++++++++++++++-- cmd/mesh-controller/release_plan.go | 8 ++--- internal/inventory/batches.go | 16 ++++++++++ internal/inventory/plans.go | 3 ++ 6 files changed, 118 insertions(+), 10 deletions(-) diff --git a/cmd/mesh-controller/batches.go b/cmd/mesh-controller/batches.go index 243c5718..4e876f0c 100644 --- a/cmd/mesh-controller/batches.go +++ b/cmd/mesh-controller/batches.go @@ -153,7 +153,11 @@ func (f following) hearMerge(ctx context.Context, m link.SourceMoved, now time.T return notNow(err) } } - if from, packaging, already := mergeCandidates(m, entries, read); len(from) == 0 && len(packaging) == 0 && len(later) == 0 { + owed, err := owedLate(ctx, inv, m, merged, touches) + if err != nil { + return notNow(err) + } + if from, packaging, already := mergeCandidates(m, entries, read); len(from) == 0 && len(packaging) == 0 && len(later) == 0 && !owed { // "Already built from it" and "nothing reads it" are different facts, and reading the first // as the second sends somebody looking for a broken trigger when the mesh is up to date. if already > 0 { @@ -216,6 +220,23 @@ func (f following) hearMerge(ctx context.Context, m link.SourceMoved, now time.T return cutBatchesHeld(ctx, f.open, now) } +// owedLate says a merge that reads as history is still the record's to answer (review of this change): a cut +// marks every module it moved seen at that moment, so a merge of that branch the bus lost and the catch-up +// hands over later — made before the cut, heard after it — reads as history and was dropped. One that touches +// a module the mesh holds, of a branch whose merges the controller keeps, made since it began keeping them, +// is kept; one made before it began is old news, as before. +func owedLate(ctx context.Context, inv *inventory.Inventory, m link.SourceMoved, merged time.Time, + touches []inventory.Entry) (bool, error) { + if len(touches) == 0 { + return false, nil + } + kept, first, err := inv.KeptOnBranch(ctx, m.Owner+"/"+m.Repo, m.Base) + if err != nil || !kept { + return false, err + } + return !merged.Before(first.Add(-mergeGrace)), nil +} + // answeredByALaterMerge is the plan that answers a merge heard after a later merge of its branch was put in // one (ADR 0276 decision 4): a batch not yet cut, whose walk plans every file of the repository's merges; an // open walk, or a walk done, that builds every module this merge would move — built from the branch after the @@ -595,7 +616,7 @@ func walkAlone(ctx context.Context, open *stores, m inventory.BatchedMerge, now p := inventory.Plan{ID: fmt.Sprintf("plan-%d", now.UnixNano()), Repository: m.Repository, Branch: m.Branch, Commit: m.Commit, Merged: m.Merged, Created: now, State: inventory.PlanQueued, Tiers: [][]string{}, Modules: map[string]*inventory.PlanModule{}, Note: "walked alone after a failed walk carried it", - Delivery: &inventory.PlanDelivery{}} + Delivery: &inventory.PlanDelivery{Alone: true}} if err := inv.SavePlan(ctx, &p); err != nil { return err } @@ -678,6 +699,7 @@ func planBatch(ctx context.Context, open *stores, batch *inventory.Plan, carry [ plan.Delivery = &inventory.PlanDelivery{} } plan.Delivery.Merges = named + plan.Delivery.Alone = batch.Delivery != nil && batch.Delivery.Alone if len(moved) == 0 { plan.State = inventory.PlanDone plan.Tiers = [][]string{} diff --git a/cmd/mesh-controller/batches_test.go b/cmd/mesh-controller/batches_test.go index b3b63de2..b2fa1802 100644 --- a/cmd/mesh-controller/batches_test.go +++ b/cmd/mesh-controller/batches_test.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "context" "encoding/json" "io" "os" @@ -138,8 +139,8 @@ func TestTwoMergesSecondsApartAreOneWalkAtTheLaterCommit(t *testing.T) { if w.Delivery == nil || !slices.Equal(w.Delivery.Merges, want) { t.Fatalf("the walk answers %+v, want %+v", w.Delivery, want) } - if len(*asked) != 2 || (*asked)[0][2] != dunst.Commit || (*asked)[1][2] != dunst.Commit { - t.Fatalf("the walk did not ask its modules at the commit it carries: %v", *asked) + if len(*asked) != 2 || (*asked)[0][2] != "main" || (*asked)[1][2] != "main" { + t.Fatalf("the walk did not ask its modules at the branch, which holds the commit it carries: %v", *asked) } // Heard again, as the bus may hand it over twice: never a second merge, nor a second walk. hear(t, open, claude, t0.Add(5*time.Minute)) @@ -625,3 +626,46 @@ func TestARemovedFileIsTheLastMergesWord(t *testing.T) { t.Fatalf("combined as %+v", got) } } + +// The catch-up hands over what the bus lost after its branch's later merges were cut, and the record keeps it: +// an earlier merge is named by the walk that carried it; a merge made before a cut and heard after it, which +// no later merge carries, joins the next batch — neither reads as history and is dropped (review of this change). +func TestTheCatchUpKeepsWhatACutMadeHistory(t *testing.T) { + open := windowed(t) + asksWithPaths(t) + ctx := t.Context() + base := time.Now().UTC().Add(-time.Hour).Truncate(time.Second) + hear(t, open, repoMerge("one", "c2", base.Add(10*time.Second)), base.Add(11*time.Second)) + cutAt(t, open, base.Add(2*time.Minute)) + ws, _ := walks(t, open) + w := ws[0] + w.State = inventory.PlanDone + if err := open.inventory.SavePlan(ctx, &w); err != nil { + t.Fatal(err) + } + lost := []link.AnnouncedMerge{ + {SourceMoved: repoMerge("one", "c1", base), At: base.Add(time.Second)}, + {SourceMoved: repoMerge("one", "c3lost", base.Add(20*time.Second)), At: base.Add(21 * time.Second)}, + } + catalogued := func(ctx context.Context) ([]inventory.Entry, map[string][]inventory.ReadRepository, error) { + entries, err := open.inventory.Catalogued(ctx) + if err != nil { + return nil, nil, err + } + read, err := readForPlanning(ctx, open.inventory) + return entries, read, err + } + if err := catchUpOnMerges(ctx, time.Now(), unheardMerges{announcedList(lost), open.inventory}, catalogued, + (following{open}).SourceMoved, t.Logf); err != nil { + t.Fatal(err) + } + got, _ := open.inventory.PlanByID(ctx, w.ID) + if len(got.Delivery.Merges) != 2 || got.Delivery.Merges[0] != (inventory.PlanMerge{Repository: "novox/one", + Commit: "c1", Carried: "c2"}) { + t.Fatalf("the earlier merge the catch-up handed over is not named by the walk that carried it: %+v", + got.Delivery.Merges) + } + if _, kept, err := open.inventory.MergeOf(ctx, "novox/one", "c3lost"); err != nil || !kept { + t.Fatalf("the merge made before the cut and heard after it was dropped: %v %v", kept, err) + } +} diff --git a/cmd/mesh-controller/missed_merges.go b/cmd/mesh-controller/missed_merges.go index 68fd3492..32ea7b30 100644 --- a/cmd/mesh-controller/missed_merges.go +++ b/cmd/mesh-controller/missed_merges.go @@ -66,6 +66,24 @@ func (u unheardMerges) AnnouncedMerges(ctx context.Context, since time.Time) ([] return out, nil } +// owedToTheRecord says a merge read as history is still owed to the record (novox/hq ADR 0276): the merges +// reader knows, when it keeps them (unheardMerges). +func owedToTheRecord(ctx context.Context, announced merges, m link.SourceMoved, entries []inventory.Entry, + read map[string][]inventory.ReadRepository) bool { + u, ok := announced.(unheardMerges) + if !ok { + return false + } + merged, err := time.Parse(time.RFC3339Nano, m.MergedAt) + if err != nil { + return false + } + raw := m + raw.MergedAt = "" + owed, err := owedLate(ctx, u.inv, m, merged, wouldMove(raw, entries, read)) + return err == nil && owed +} + // catchingUpOnMerges reads back the forge's announcements on a timer, until the context ends. func catchingUpOnMerges(ctx context.Context, open *stores, announced merges) { f := following{open} @@ -136,7 +154,7 @@ func catchUpOnMerges(ctx context.Context, now time.Time, announced merges, if now.Sub(a.At) < mergeGrace { continue } - if len(wouldMove(a.SourceMoved, entries, read)) == 0 { + if len(wouldMove(a.SourceMoved, entries, read)) == 0 && !owedToTheRecord(ctx, announced, a.SourceMoved, entries, read) { continue } if entries, read, err = catalogued(ctx); err != nil { @@ -144,7 +162,12 @@ func catchUpOnMerges(ctx context.Context, now time.Time, announced merges, } moves := wouldMove(a.SourceMoved, entries, read) if len(moves) == 0 { - continue + if !owedToTheRecord(ctx, announced, a.SourceMoved, entries, read) { + continue + } + raw := a.SourceMoved + raw.MergedAt = "" + moves = wouldMove(raw, entries, read) } var names []string for _, e := range moves { diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 989d3449..6b238e9d 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -330,12 +330,12 @@ func askModule(ctx context.Context, p *inventory.Plan, name string, byName map[s } source := buildSource{Repository: e.Source.Repository, Seat: e.Source.Seat} fmt.Printf(" tier %d: ", p.Tier) - // The branch it follows, never a commit a build once named (novox/hq 04-ISSUES/215) — but for the commit a - // batch's walk carries of the module's own repository and branch (novox/hq ADR 0276): the walk builds what - // it answers, not a merge heard after it was cut, and a merge walked alone is built on its own commit. + // The branch it follows, never a commit a build once named (novox/hq 04-ISSUES/215): the branch contains every + // commit a batch's walk carries — but for a merge walked alone after a failed walk (novox/hq ADR 0276 + // decision 3), built on its own commit to find which merge brought the failure. ref := followedBranch(e.Source.Ref) carried := p.CommitOn(e.Source.Repository, ref) - if len(p.Commits) > 0 && carried != "" { + if p.Delivery != nil && p.Delivery.Alone && carried != "" { ref = carried } id, err := askABuild(ctx, source, e.Source.Path, ref) diff --git a/internal/inventory/batches.go b/internal/inventory/batches.go index 1eae7825..1d73228e 100644 --- a/internal/inventory/batches.go +++ b/internal/inventory/batches.go @@ -64,6 +64,22 @@ func (i *Inventory) LaterMergesOf(ctx context.Context, repository, branch string order by merged_at desc`, repository, branch, after) } +// KeptOnBranch says the controller keeps a merge of a repository's branch, and when it heard its first merge +// of any: a merge of a branch it keeps merges of, made since then, is the record's to answer, whatever a +// module's last look says (novox/hq ADR 0276). +func (i *Inventory) KeptOnBranch(ctx context.Context, repository, branch string) (bool, time.Time, error) { + var first *time.Time + var kept bool + err := i.store.Pool().QueryRow(ctx, + `select (select min(heard_at) from batched_merge), + exists (select 1 from batched_merge where repository = lower($1) and branch = $2)`, + repository, branch).Scan(&first, &kept) + if err != nil || first == nil { + return false, time.Time{}, err + } + return kept, first.UTC(), nil +} + // AloneMerges is every merge waiting to be walked on its own commit, the newest first (ADR 0276 decision 3). func (i *Inventory) AloneMerges(ctx context.Context) ([]BatchedMerge, error) { return i.merges(ctx, `where plan_id is null and alone order by merged_at desc nulls last, heard_at desc`) diff --git a/internal/inventory/plans.go b/internal/inventory/plans.go index e43186d5..4dba027c 100644 --- a/internal/inventory/plans.go +++ b/internal/inventory/plans.go @@ -152,6 +152,9 @@ type PlanDelivery struct { TakenOverBy string `json:"taken_over_by,omitempty"` // Batch is the window of a batch; nil once it is cut into a walk. Batch *PlanBatch `json:"batch,omitempty"` + // Alone says the walk walks one merge on its own commit, after a failed walk carried it in a later one: + // built at that commit, not at the branch. + Alone bool `json:"alone,omitempty"` } // Waiting is whether the walk waits for its delivery's word.