diff --git a/cmd/mesh-controller/catch_up.go b/cmd/mesh-controller/catch_up.go index 5cb80a9f..21e9fd19 100644 --- a/cmd/mesh-controller/catch_up.go +++ b/cmd/mesh-controller/catch_up.go @@ -103,7 +103,10 @@ func readTrunkMerges(ctx context.Context, owner, repo, branch string) ([]trunkMe break } for _, c := range commits { - if c.SecondParent != "" { + // A merge of the trunk is a pull request's merge into it. A commit with a second parent that is none is a merge made + // inside some branch ("Merge remote-tracking branch 'origin/main' into fix/..."), which the trunk's history lists too and + // which no Approve ever covered: read as a gap it would raise a condition for nothing a person decides. + if c.SecondParent != "" && mergesIntoTrunk(c.Subject, branch) { merges = append(merges, trunkMerge{Commit: c.Commit, Head: c.SecondParent, Subject: c.Subject, At: c.At}) } } @@ -136,6 +139,12 @@ func pullOfSubject(subject string) (title string, number int, ok bool) { return m[1], n, true } +// mergesIntoTrunk says a commit's subject is a pull request's merge into this trunk. +func mergesIntoTrunk(subject, branch string) bool { + m := mergeSubject.FindStringSubmatch(strings.TrimSpace(subject)) + return m != nil && m[4] == branch +} + // headBranchOfSubject is the branch a merge commit's subject says was merged. func headBranchOfSubject(subject string) string { if m := mergeSubject.FindStringSubmatch(strings.TrimSpace(subject)); m != nil { diff --git a/cmd/mesh-controller/catch_up_test.go b/cmd/mesh-controller/catch_up_test.go index 2e2ec779..8dca2290 100644 --- a/cmd/mesh-controller/catch_up_test.go +++ b/cmd/mesh-controller/catch_up_test.go @@ -34,7 +34,8 @@ func TestOnlyTheLastFiftyMergesOfATrunkAreRead(t *testing.T) { pages := map[int][]trunkCommit{} var all []trunkCommit // newest first: 51 merges, a plain commit between each pair for i := 51; i >= 1; i-- { - all = append(all, trunkCommit{Commit: fmt.Sprintf("merge-%02d", i), SecondParent: fmt.Sprintf("head-%02d", i)}, + all = append(all, trunkCommit{Commit: fmt.Sprintf("merge-%02d", i), SecondParent: fmt.Sprintf("head-%02d", i), + Subject: fmt.Sprintf("Merge pull request 'change %d' (#%d) from fix/x into main", i, i)}, trunkCommit{Commit: fmt.Sprintf("plain-%02d", i)}) } for i := 0; i < len(all); i += 50 { @@ -264,7 +265,8 @@ func TestAShortTrunkIsReadWholeAndAnEmptyOneIsNoMerges(t *testing.T) { if page > 1 { return nil, nil } - return []trunkCommit{{Commit: "m2", SecondParent: "h2"}, {Commit: "p"}, {Commit: "m1", SecondParent: "h1"}}, nil + return []trunkCommit{{Commit: "m2", SecondParent: "h2", Subject: "Merge pull request 'two' (#2) from fix/b into main"}, + {Commit: "p"}, {Commit: "m1", SecondParent: "h1", Subject: "Merge pull request 'one' (#1) from fix/a into main"}}, nil } got, err := readTrunkMerges(t.Context(), "novox", "x", "main") if err != nil || len(got) != 2 || got[0].Commit != "m1" || got[1].Commit != "m2" { @@ -292,3 +294,26 @@ func TestTheConditionOfAMergeNotHeardIsInPlainWords(t *testing.T) { t.Fatalf("the condition's words are not plain: %s\n%q\n%q\n%q", why, c.Headline, c.Explanation, c.Needs) } } + +// The trunk's history lists the merges made inside a pull request's branch too. On novox (2026-10-20) 15 of the 15 non-pull-request +// "gaps" of mesh-catalog and mesh-controller were such merges; only a pull request's merge into this trunk is a merge to catch up. +func TestAMergeMadeInsideABranchIsNoMergeOfTheTrunk(t *testing.T) { + was := trunkCommitsPage + t.Cleanup(func() { trunkCommitsPage = was }) + trunkCommitsPage = func(_ context.Context, _, _, _ string, page int) ([]trunkCommit, error) { + if page > 1 { + return nil, nil + } + return []trunkCommit{ + {Commit: "c0880c4d", SecondParent: "b9104454", Subject: "Merge pull request 'Read the grant file' (#378) from fix/1332 into main"}, + {Commit: "6394a62a", SecondParent: "x1", Subject: "Merge remote-tracking branch 'origin/main' into fix/1081-withdraw"}, + {Commit: "896f1d28", SecondParent: "x2", Subject: "Merge feat/agents-write-git-ssh into feat/agents-write-git-ssh-f (issue 955 F)"}, + {Commit: "4d03755c", SecondParent: "x3", Subject: "Bring the branch up to main after #354 (issue 1047)"}, + {Commit: "other", SecondParent: "x4", Subject: "Merge pull request 'Elsewhere' (#5) from fix/y into nox-mesh"}, + }, nil + } + got, err := readTrunkMerges(t.Context(), "novox", "mesh-catalog", "main") + if err != nil || len(got) != 1 || got[0].Commit != "c0880c4d" { + t.Fatalf("read %+v %v: only keycloak's pull request merge into main is a merge to catch up", got, err) + } +}