From 3045af47223fc2c9580cf8a662ea810d172f0d46 Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Tue, 20 Oct 2026 14:38:47 +0000 Subject: [PATCH] A merge from before the controller kept merges of the trunk is history, never a gap (issue 1336) --- cmd/mesh-controller/catch_up.go | 31 ++++++++++++++ .../catch_up_integration_test.go | 42 +++++++++++++++++++ cmd/mesh-controller/catch_up_test.go | 24 +++++++++++ 3 files changed, 97 insertions(+) diff --git a/cmd/mesh-controller/catch_up.go b/cmd/mesh-controller/catch_up.go index 21e9fd19..0a4d6619 100644 --- a/cmd/mesh-controller/catch_up.go +++ b/cmd/mesh-controller/catch_up.go @@ -114,6 +114,21 @@ 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. +func sinceTheRecord(merges []trunkMerge, kept bool, first time.Time) []trunkMerge { + if !kept { + return nil + } + var out []trunkMerge + for _, m := range merges { + if !m.At.Before(first.Add(-mergeGrace)) { + out = append(out, m) + } + } + return out +} + // lastMerges is the newest catchUpWindow of merges, given newest first, oldest first. func lastMerges(newestFirst []trunkMerge) []trunkMerge { if len(newestFirst) > catchUpWindow { @@ -478,6 +493,22 @@ 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 + } + 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) + } 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 ac9cf58e..3a9ddb72 100644 --- a/cmd/mesh-controller/catch_up_integration_test.go +++ b/cmd/mesh-controller/catch_up_integration_test.go @@ -78,6 +78,13 @@ 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). +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)) +} + func mergeKnown(t *testing.T, open *stores, commit string) bool { t.Helper() _, known, err := open.inventory.MergeOf(t.Context(), "novox/mesh-catalog", commit) @@ -197,6 +204,7 @@ func TestAGapWithNoApproveRaisesOneConditionKeptOpenAndClearedWhenPlanned(t *tes open := windowed(t) everyHeadWaits(t) asksRecorded(t) + aKeptMergeBefore(t, open) keeper, _ := withConditionsInMemory(t) tr := aTrunkOf(t, "mesh-catalog", "notes") tr.approved["mesh-catalog-c01"] = false @@ -242,6 +250,7 @@ func TestAForgeThatDoesNotAnswerLeavesTheCatchUpForTheNextStart(t *testing.T) { open := windowed(t) everyHeadWaits(t) asksRecorded(t) + aKeptMergeBefore(t, open) tr := aTrunkOf(t, "mesh-catalog", "notes") tr.silent = true if later := (following{open: open}).catchUp(t.Context(), nil); !later { @@ -262,6 +271,7 @@ func TestAGapThatAddsAModuleIsHeardOnItsWarrantAndRaisedWithout(t *testing.T) { open := windowed(t) everyHeadWaits(t) asksRecorded(t) + aKeptMergeBefore(t, open) keeper, _ := withConditionsInMemory(t) tr := aTrunkOf(t, "mesh-catalog", "newmodule") tr.approved["mesh-catalog-c01"] = false @@ -289,3 +299,35 @@ func TestAGapThatAddsAModuleIsHeardOnItsWarrantAndRaisedWithout(t *testing.T) { t.Fatal("a merge adding a module was not heard once its Approve was on record") } } + +// A trunk with merges from before the controller began keeping merges, and merges since: only the later ones are gaps. +func TestAMergeFromBeforeTheRecordBeganIsHistoryAndNoCondition(t *testing.T) { + open := windowed(t) + everyHeadWaits(t) + asksRecorded(t) + keeper, _ := withConditionsInMemory(t) + tr := aTrunkOf(t, "mesh-catalog", "notes", "notes", "notes") + tr.approved["mesh-catalog-c01"], tr.approved["mesh-catalog-c02"], tr.approved["mesh-catalog-c03"] = false, false, false + // The record begins at the second merge: the first is two days older. + tr.merges[0].At = t0.Add(-48 * time.Hour) + hear(t, open, catalogueMerge("mesh-catalog-c02", "notes", t0.Add(time.Minute)), t0.Add(time.Minute)) + delete(tr.approved, "mesh-catalog-c02") + (following{open: open}).catchUp(t.Context(), nil) + // The first merge (two days before the record) was not even read from the forge: only the third, which is unknown, was. + if tr.events != 1 { + t.Fatalf("%d merge(s) were read from the forge: the one from before the record is history and is not read", tr.events) + } + all, err := keeper.Open(t.Context()) + if err != nil { + t.Fatal(err) + } + var raised int + for _, c := range all { + if c.Kind == kindMergeNotHeard { + raised++ + } + } + if raised != 1 { + t.Fatalf("%d condition(s) raised: only the merge since the record began (the third) is a gap", raised) + } +} diff --git a/cmd/mesh-controller/catch_up_test.go b/cmd/mesh-controller/catch_up_test.go index 8dca2290..ed4c1cac 100644 --- a/cmd/mesh-controller/catch_up_test.go +++ b/cmd/mesh-controller/catch_up_test.go @@ -317,3 +317,27 @@ func TestAMergeMadeInsideABranchIsNoMergeOfTheTrunk(t *testing.T) { t.Fatalf("read %+v %v: only keycloak's pull request merge into main is a merge to catch up", got, err) } } + +// 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)}, + } + got := sinceTheRecord(merges, true, first) + var commits []string + for _, m := range got { + commits = append(commits, m.Commit) + } + 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)) + } +}