From 9f0d9098b7359b8a21e51ec1d62043ec0d373058 Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Tue, 20 Oct 2026 14:49:10 +0000 Subject: [PATCH] Begin the record of merges at the earliest merge kept on any trunk, not per trunk (ADR 0463 section 2, issue 1336) --- cmd/mesh-controller/catch_up.go | 38 +++++++++-------- .../catch_up_integration_test.go | 23 ++++++++++- cmd/mesh-controller/catch_up_test.go | 41 ++++++++++--------- internal/inventory/batches.go | 10 +++++ internal/inventory/chain_rollouts.go | 18 ++++++++ internal/inventory/chain_rollouts_test.go | 25 +++++++++++ 6 files changed, 116 insertions(+), 39 deletions(-) diff --git a/cmd/mesh-controller/catch_up.go b/cmd/mesh-controller/catch_up.go index 0a4d6619..f3f40bb7 100644 --- a/cmd/mesh-controller/catch_up.go +++ b/cmd/mesh-controller/catch_up.go @@ -114,8 +114,10 @@ func readTrunkMerges(ctx context.Context, owner, repo, branch string) ([]trunkMe return lastMerges(merges), nil } -// sinceTheRecord are the merges that can be gaps: those made since the controller began keeping merges of the trunk (first, less -// mergeGrace, as owedLate judges). With no merge kept on the trunk yet there is no record to have a gap in, and none is. +// sinceTheRecord are the merges that can be gaps (novox/hq ADR 0463 §2): those made since the controller's record of merges begins on +// the mesh (first, the earliest merge it holds of any repository and branch, less mergeGrace as owedLate judges). A merge before it is +// history on every trunk, and a trunk with no merge kept of its own still has the merges made since as gaps. With no merge kept +// anywhere there is no record to have a gap in. func sinceTheRecord(merges []trunkMerge, kept bool, first time.Time) []trunkMerge { if !kept { return nil @@ -469,6 +471,17 @@ func (f following) catchUp(ctx context.Context, only *caughtTrunk) (later bool) fmt.Printf("catch-up: the catalogue cannot be read, so no merge is caught up now: %v\n", err) return true } + kept, first, err := inv.EarliestKept(ctx) + if err != nil { + fmt.Printf("catch-up: what the controller kept cannot be read, so no merge is caught up now: %v\n", err) + return true + } + if kept { + fmt.Printf("catch-up: the controller's record of merges begins %s; a merge made before it, less %s, is history on every trunk\n", + first.UTC().Format(time.RFC3339), mergeGrace) + } else { + fmt.Println("catch-up: the controller holds no merge of any trunk yet, so none can have been missed") + } trunks := trunksOfCatalogue(newSourceForms(ctx, inv), entries) if only != nil { trunks = slices.DeleteFunc(trunks, func(t caughtTrunk) bool { @@ -476,14 +489,14 @@ func (f following) catchUp(ctx context.Context, only *caughtTrunk) (later bool) }) } for _, t := range trunks { - if f.catchUpTrunk(ctx, t) { + if f.catchUpTrunk(ctx, t, kept, first) { later = true } } return later } -func (f following) catchUpTrunk(ctx context.Context, t caughtTrunk) (later bool) { +func (f following) catchUpTrunk(ctx context.Context, t caughtTrunk, kept bool, first time.Time) (later bool) { inv := f.open.inventory repository := t.repository() merges, err := readTrunkMerges(ctx, t.Owner, t.Repo, t.Branch) @@ -493,22 +506,11 @@ func (f following) catchUpTrunk(ctx context.Context, t caughtTrunk) (later bool) "or the next merge heard there does it: %v\n", repository, t.Branch, err) return true } - // **A merge from before the controller kept merges of this trunk is history, never a gap**: it held no record then, so it cannot - // have missed it (ADR 0457, as corrected). The first it kept, less the grace owedLate uses, is where the record begins. - kept, first, err := inv.KeptOnBranch(ctx, repository, t.Branch) - if err != nil { - fmt.Printf("catch-up: what the controller kept of %s %s cannot be read, so it is not caught up now: %v\n", repository, t.Branch, err) - return true - } + // **A merge made before the controller's record of merges begins is history, never a gap** (novox/hq ADR 0463 §2). read := len(merges) merges = sinceTheRecord(merges, kept, first) - if kept { - fmt.Printf("catch-up: %s %s: the record begins %s; %d of the last %d pull request merges were made since\n", repository, t.Branch, - first.UTC().Format(time.RFC3339), len(merges), read) - } else { - fmt.Printf("catch-up: %s %s: the controller kept no merge of it yet, so none of its %d pull request merges can have been missed\n", - repository, t.Branch, read) - } + fmt.Printf("catch-up: %s %s: %d of the last %d pull request merges were made since the record began\n", repository, t.Branch, + len(merges), read) unwarranted := map[string]bool{} for _, tm := range merges { id := fmt.Sprintf("%s_%.8s", repository, tm.Commit) diff --git a/cmd/mesh-controller/catch_up_integration_test.go b/cmd/mesh-controller/catch_up_integration_test.go index 3a9ddb72..c0b698fc 100644 --- a/cmd/mesh-controller/catch_up_integration_test.go +++ b/cmd/mesh-controller/catch_up_integration_test.go @@ -78,8 +78,8 @@ func aTrunkOf(t *testing.T, repo string, modules ...string) *aTrunk { return tr } -// aKeptMergeBefore is a merge the controller heard on the catalogue's trunk before the merges of a test: where its record begins, -// so what the trunk holds from then on can be a gap (ADR 0457: a merge from before it cannot have been missed). +// aKeptMergeBefore is a merge the controller heard before the merges of a test: where its record begins, so what a trunk holds from +// then on can be a gap (novox/hq ADR 0463 §2). func aKeptMergeBefore(t *testing.T, open *stores) { t.Helper() hear(t, open, catalogueMerge("mesh-catalog-seed", "app", t0.Add(-time.Hour)), t0.Add(-time.Hour)) @@ -331,3 +331,22 @@ func TestAMergeFromBeforeTheRecordBeganIsHistoryAndNoCondition(t *testing.T) { t.Fatalf("%d condition(s) raised: only the merge since the record began (the third) is a gap", raised) } } + +// novox/hq ADR 0463 §2: the record begins at the earliest merge kept on ANY trunk. A trunk the controller kept no merge of has the +// merges made since as gaps (mesh-delivery #1 on novox), and the merges made before as history. +func TestATrunkWithNoKeptMergeHasItsLaterMergesAsGapsAndItsEarlierOnesAsHistory(t *testing.T) { + open := windowed(t) + everyHeadWaits(t) + asksRecorded(t) + // The record begins on another trunk, at t0 less an hour. + hear(t, open, repoMerge("one", "c-one", t0.Add(-time.Hour)), t0.Add(-time.Hour)) + tr := aTrunkOf(t, "mesh-catalog", "notes", "notes") + tr.merges[0].At = t0.Add(-72 * time.Hour) // before the record: history + (following{open: open}).catchUp(t.Context(), nil) + if mergeKnown(t, open, "mesh-catalog-c01") { + t.Fatal("a merge from before the record began was heard") + } + if !mergeKnown(t, open, "mesh-catalog-c02") { + t.Fatal("a merge since the record began, on a trunk with no merge kept, was not caught up") + } +} diff --git a/cmd/mesh-controller/catch_up_test.go b/cmd/mesh-controller/catch_up_test.go index ed4c1cac..b120a1cd 100644 --- a/cmd/mesh-controller/catch_up_test.go +++ b/cmd/mesh-controller/catch_up_test.go @@ -318,26 +318,29 @@ func TestAMergeMadeInsideABranchIsNoMergeOfTheTrunk(t *testing.T) { } } -// A merge from before the controller kept merges of the trunk is history: it held no record then, so it cannot have missed it. On -// novox (2026-10-20) about 95 of the 110 named gaps were merges of 2026-09-30 to 2026-10-14, before the record began. -func TestOnlyTheMergesSinceTheControllerBeganKeepingMergesAreGaps(t *testing.T) { - first := time.Date(2026, 10, 17, 12, 0, 0, 0, time.UTC) // the first merge it heard on this trunk - merges := []trunkMerge{ - {Commit: "old-1", At: first.Add(-30 * 24 * time.Hour)}, - {Commit: "old-2", At: first.Add(-2 * time.Hour)}, - {Commit: "within-grace", At: first.Add(-mergeGrace / 2)}, - {Commit: "the-first", At: first}, - {Commit: "later", At: first.Add(48 * time.Hour)}, +// novox/hq ADR 0463 §2: the record of merges begins at the earliest merge the controller holds on any trunk. A merge before it is +// history on every trunk; a trunk with no merge kept of its own still has the merges made since as gaps (mesh-delivery #1 on novox). +func TestOnlyTheMergesSinceTheRecordBeganAreGapsOnEveryTrunk(t *testing.T) { + first := time.Date(2026, 10, 17, 12, 0, 0, 0, time.UTC) // the first merge it heard, on some trunk of the mesh + trunk := func(prefix string) []trunkMerge { + return []trunkMerge{ + {Commit: prefix + "-old-1", At: first.Add(-30 * 24 * time.Hour)}, + {Commit: prefix + "-old-2", At: first.Add(-2 * time.Hour)}, + {Commit: prefix + "-within-grace", At: first.Add(-mergeGrace / 2)}, + {Commit: prefix + "-since", At: first.Add(24 * time.Hour)}, + } } - got := sinceTheRecord(merges, true, first) - var commits []string - for _, m := range got { - commits = append(commits, m.Commit) + // Two trunks, one of which the controller has kept no merge of: both read by the one starting point. + for _, prefix := range []string{"kept-trunk", "trunk-with-no-kept-merge"} { + var commits []string + for _, m := range sinceTheRecord(trunk(prefix), true, first) { + commits = append(commits, m.Commit) + } + if want := prefix + "-within-grace " + prefix + "-since"; strings.Join(commits, " ") != want { + t.Errorf("%s: the gaps are %v, want %q: earlier ones are history on every trunk, later ones are gaps", prefix, commits, want) + } } - if want := "within-grace the-first later"; strings.Join(commits, " ") != want { - t.Fatalf("the gaps are %v, want %q: nothing before the record can have been missed", commits, want) - } - if none := sinceTheRecord(merges, false, time.Time{}); len(none) != 0 { - t.Fatalf("a trunk the controller kept no merge of has %d gap(s), but no record to have one in", len(none)) + if none := sinceTheRecord(trunk("x"), false, time.Time{}); len(none) != 0 { + t.Errorf("with no merge kept anywhere there are %d gap(s), but no record to have one in", len(none)) } } diff --git a/internal/inventory/batches.go b/internal/inventory/batches.go index e870a676..7ca63dce 100644 --- a/internal/inventory/batches.go +++ b/internal/inventory/batches.go @@ -75,6 +75,16 @@ func (i *Inventory) KeptOnBranch(ctx context.Context, repository, branch string) return i.chainKeptOnBranch(ctx, c, repository, branch) } +// EarliestKept says the controller keeps a merge of some repository's branch, and when it heard the first of all: where its record of +// merges begins on the whole mesh (novox/hq ADR 0463). A merge made before then is history on every trunk. +func (i *Inventory) EarliestKept(ctx context.Context) (bool, time.Time, error) { + c, err := i.theChain(ctx) + if err != nil { + return false, time.Time{}, err + } + return i.chainEarliestKept(ctx, c) +} + // 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) { c, err := i.theChain(ctx) diff --git a/internal/inventory/chain_rollouts.go b/internal/inventory/chain_rollouts.go index 6c242b6d..3988f558 100644 --- a/internal/inventory/chain_rollouts.go +++ b/internal/inventory/chain_rollouts.go @@ -437,6 +437,24 @@ func (i *Inventory) chainKeptOnBranch(ctx context.Context, c *chain.Chain, repos return true, first.UTC(), nil } +// chainEarliestKept is when the controller heard the first merge it holds a record of, on any repository and branch. +func (i *Inventory) chainEarliestKept(ctx context.Context, c *chain.Chain) (bool, time.Time, error) { + w, err := i.chainWinners(ctx, c) + if err != nil { + return false, time.Time{}, err + } + var first time.Time + for _, h := range w { + if first.IsZero() || h.m.Heard.Before(first) { + first = h.m.Heard + } + } + if first.IsZero() { + return false, time.Time{}, nil + } + return true, first.UTC(), nil +} + func (i *Inventory) chainAloneMerges(ctx context.Context, c *chain.Chain) ([]BatchedMerge, error) { w, err := i.chainWinners(ctx, c) if err != nil { diff --git a/internal/inventory/chain_rollouts_test.go b/internal/inventory/chain_rollouts_test.go index fffc8576..d7e8b7f3 100644 --- a/internal/inventory/chain_rollouts_test.go +++ b/internal/inventory/chain_rollouts_test.go @@ -347,3 +347,28 @@ func TestThePlansAreReadFromAViewKeptByOneWatch(t *testing.T) { return len(open) == 3 }) } + +// Where the record of merges begins is the earliest merge kept on any repository and branch (novox/hq ADR 0463 §2). +func TestTheRecordOfMergesBeginsAtTheEarliestMergeKeptOnAnyTrunk(t *testing.T) { + inv, c := anInventoryOnTheChain(t) + ctx := context.Background() + if kept, _, err := inv.chainEarliestKept(ctx, c); err != nil || kept { + t.Fatalf("an inventory with no merge kept says it keeps one: %v %v", kept, err) + } + if err := inv.chainSavePlans(ctx, c, []*Plan{aBatch("plan-1", plansAt), aBatch("plan-2", plansAt)}, nil); err != nil { + t.Fatal(err) + } + later := aMerge("m-late", "plan-1") + later.Repository, later.Branch, later.Heard = "novox/mesh-sdk", "main", plansAt.Add(48*time.Hour) + earlier := aMerge("m-early", "plan-2") + earlier.Repository, earlier.Branch, earlier.Heard = "novox/photos", "nox-mesh", plansAt.Add(time.Hour) + for _, m := range []BatchedMerge{later, earlier} { + if added, err := inv.chainAddMerge(ctx, c, m); err != nil || !added { + t.Fatalf("%s: %v %v", m.Commit, added, err) + } + } + kept, first, err := inv.chainEarliestKept(ctx, c) + if err != nil || !kept || !first.Equal(plansAt.Add(time.Hour)) { + t.Fatalf("the record begins at %v (%v, %v), want %v", first, kept, err, plansAt.Add(time.Hour)) + } +}