diff --git a/modules/mesh-delivery-indexer/go.mod b/modules/mesh-delivery-indexer/go.mod index 0aaff15..5f87256 100644 --- a/modules/mesh-delivery-indexer/go.mod +++ b/modules/mesh-delivery-indexer/go.mod @@ -3,7 +3,7 @@ module mesh-delivery-indexer go 1.26.0 require ( - git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261023150321-a2c813a6b468 + git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261024231716-571ac521f053 github.com/nats-io/nats-server/v2 v2.11.17 github.com/nats-io/nats.go v1.54.0 ) diff --git a/modules/mesh-delivery-indexer/go.sum b/modules/mesh-delivery-indexer/go.sum index 2a3e9f3..c1bfa2e 100644 --- a/modules/mesh-delivery-indexer/go.sum +++ b/modules/mesh-delivery-indexer/go.sum @@ -1,5 +1,7 @@ git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261023150321-a2c813a6b468 h1:UcMee+IV2ii5E4dRZdQUaSaeh9IedTFYumuQbBiB0k0= git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261023150321-a2c813a6b468/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261024231716-571ac521f053 h1:8Kbp3aJG3jGndDOfmGdMSDgJVEcc1hyu921hVnnlnlQ= +git.novox.be/novox/mesh-sdk/go v0.1.14-0.20261024231716-571ac521f053/go.mod h1:ouERNpZizzwZ0HLEtAypsY9FR20FyUgUYRni6J7kVCY= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op h1:Z/MZK75wC/NSrkgqeNIa7jexam9uWzhLmFTSCPI/kn0= github.com/antithesishq/antithesis-sdk-go v0.7.0-default-no-op/go.mod h1:FQyySiasQQM8735Ddel3MRojmy4dA1IqCeyJ5jmPMbI= github.com/google/go-tpm v0.9.8 h1:slArAR9Ft+1ybZu0lBwpSmpwhRXaa85hWtMinMyRAWo= diff --git a/modules/mesh-delivery-indexer/internal/index/candidates.go b/modules/mesh-delivery-indexer/internal/index/candidates.go index 7a14378..b9f9f21 100644 --- a/modules/mesh-delivery-indexer/internal/index/candidates.go +++ b/modules/mesh-delivery-indexer/internal/index/candidates.go @@ -71,7 +71,7 @@ func (ix *Indexer) logf(format string, args ...any) { } // Apply takes one message of the bridge's stream FORGE into the group of its branch. A pull message adds or moves the -// pull request, a closed one or a merged message takes it out; a group left with none is deleted. A change is written +// pull request, a closed one takes it out, a merged one marks it merged; a group left with none open is deleted. A change is written // by compare-and-set on the revision read, and read again on a conflict. A lone pull request is a group of one. A // merged message also writes the fingerprint of each module of its repository (modules.go). func (ix *Indexer) Apply(ctx context.Context, f delivery.Forge) error { @@ -154,12 +154,13 @@ func (ix *Indexer) applyOpen(ctx context.Context, f delivery.Forge) error { return nil, false, fmt.Errorf("candidates.%s holds what is not a candidate: %w", key, err) } kept := slices.DeleteFunc(slices.Clone(c.Pulls), func(p delivery.CandidatePull) bool { - return !named[delivery.OpenPull{Repository: p.Repository, Number: p.Number}] + // a member that merged is open no more, and stays while another member is (issue 1791) + return !p.Merged && !named[delivery.OpenPull{Repository: p.Repository, Number: p.Number}] }) switch { case len(kept) == len(c.Pulls): return nil, false, nil - case len(kept) == 0: + case !anyOpen(kept): ix.logf("%s: no pull request of the group is open, so no candidate", key) return nil, true, nil } @@ -232,7 +233,16 @@ func (ix *Indexer) next(cur *delivery.Candidate, f delivery.Forge, digests map[s if at < 0 { return cur } - pulls = slices.Delete(pulls, at, at+1) + if f.Kind == delivery.ForgeMerged { + // A member that merges stays, marked merged, with the head it merged at: the heads hash the group's Approve + // covers holds, so the checker starts no new check for the members still open (issue 1791; issue 1921 S9). + pulls[at].Merged = true + } else { + pulls = slices.Delete(pulls, at, at+1) + } + if !anyOpen(pulls) { + return nil // every member merged or closed: the group is done + } } else { entry := delivery.CandidatePull{Head: delivery.Head{Repository: f.Repository, Number: f.Number, Head: f.Head}, Modules: []string{}} switch { @@ -276,6 +286,11 @@ func (ix *Indexer) next(cur *delivery.Candidate, f delivery.Forge, digests map[s return &delivery.Candidate{V: delivery.Version, Group: f.Branch, Base: base, HeadsHash: hash, Pulls: pulls, FPSet: delivery.FingerprintSet(pulls)} } +// anyOpen says whether a group's pull requests hold one that has not merged. +func anyOpen(pulls []delivery.CandidatePull) bool { + return slices.ContainsFunc(pulls, func(p delivery.CandidatePull) bool { return !p.Merged }) +} + // NoneNoPreview is why a candidate's touched module has no fingerprint of the new form: the merge preview is not // computed in this release (novox/hq ADR 0465 clause 3; the checker comes with issue 1445). const NoneNoPreview = "no merge preview is computed" diff --git a/modules/mesh-delivery-indexer/internal/index/index_test.go b/modules/mesh-delivery-indexer/internal/index/index_test.go index 74bcc57..3ba9707 100644 --- a/modules/mesh-delivery-indexer/internal/index/index_test.go +++ b/modules/mesh-delivery-indexer/internal/index/index_test.go @@ -206,8 +206,8 @@ func TestDirsNotKnownKeepTheModulesOfTheSameHead(t *testing.T) { // 1238/AC4 } } -func TestClosedAndMergedTakeAPullOutAndTheLastDeletesTheKey(t *testing.T) { // 1238/AC5 - for _, how := range []string{"closed", "merged"} { +func TestClosedTakesAPullOutAndTheLastDeletesTheKey(t *testing.T) { // 1238/AC5 (a merge: TestAMergedMemberStays...) + for _, how := range []string{"closed"} { ix, st, _ := newIndexer(t) ctx := context.Background() _ = ix.Apply(ctx, pull("novox/mesh-delivery", 12, "feat/x", h1, "modules/a")) @@ -387,3 +387,70 @@ func TestAnOpenMessageDropsEveryCandidateItDoesNotName(t *testing.T) { // 1279/A t.Errorf("after an empty open message the candidates hold %v", st.vals) } } + +// 1791 (issue 1921, try speed S9): a member of a group that merges while another is open stays in the candidate, marked +// merged, with its head: the heads hash the Approve covers holds, so no new check starts for the members left. An open +// message does not drop it, a close of the last open member deletes the group, and the last merge does too. +func TestAMergedMemberStaysInItsGroupUntilEveryMemberMerged(t *testing.T) { + merged := func(repo string, n int, head string) delivery.Forge { + return delivery.Forge{V: delivery.Version, Kind: delivery.ForgeMerged, Repository: repo, Branch: "feat/x", Number: n, Head: head, Merge: h3, Root: "r", Trees: map[string]string{"a": "t"}} + } + ix, st, _ := newIndexer(t) + ctx := context.Background() + for _, f := range []delivery.Forge{pull("novox/mesh-delivery", 12, "feat/x", h1, "modules/a"), pull("novox/mesh-catalog", 300, "feat/x", h2, "modules/c"), + pull("novox/mesh-sdk", 7, "feat/x", h3)} { + if err := ix.Apply(ctx, f); err != nil { + t.Fatal(err) + } + } + before, _ := read(t, st, "feat/x") + if err := ix.Apply(ctx, merged("novox/mesh-sdk", 7, h3)); err != nil { + t.Fatal(err) + } + if err := ix.Apply(ctx, merged("novox/mesh-delivery", 12, h1)); err != nil { + t.Fatal(err) + } + c, ok := read(t, st, "feat/x") + if !ok || len(c.Pulls) != 3 || c.HeadsHash != before.HeadsHash { + t.Fatalf("after two merges the candidate = %+v (kept %v), want the three members and the same heads hash", c, ok) + } + for _, p := range c.Pulls { + if want := p.Repository != "novox/mesh-catalog"; p.Merged != want { + t.Errorf("%s#%d merged = %v, want %v", p.Repository, p.Number, p.Merged, want) + } + } + // The open message names only the member still open: the merged ones stay. + if err := ix.Apply(ctx, open(delivery.OpenPull{Repository: "novox/mesh-catalog", Number: 300})); err != nil { + t.Fatal(err) + } + if c, _ := read(t, st, "feat/x"); len(c.Pulls) != 3 || c.HeadsHash != before.HeadsHash { + t.Errorf("the open message dropped a merged member: %+v", c) + } + // The last open member merges: the group is done. + if err := ix.Apply(ctx, merged("novox/mesh-catalog", 300, h2)); err != nil { + t.Fatal(err) + } + if _, ok := read(t, st, "feat/x"); ok { + t.Error("every member merged and the group is still a candidate") + } + // A close of the last open member deletes the group too. + ix, st, _ = newIndexer(t) + _ = ix.Apply(ctx, pull("novox/mesh-delivery", 12, "feat/x", h1, "modules/a")) + _ = ix.Apply(ctx, pull("novox/mesh-catalog", 300, "feat/x", h2, "modules/c")) + _ = ix.Apply(ctx, merged("novox/mesh-delivery", 12, h1)) + closed := pull("novox/mesh-catalog", 300, "feat/x", h2) + closed.Action = delivery.PullClosed + if err := ix.Apply(ctx, closed); err != nil { + t.Fatal(err) + } + if _, ok := read(t, st, "feat/x"); ok { + t.Error("the last open member closed and the group, holding only a merged one, is still a candidate") + } + // A lone pull request that merges leaves no group. + ix, st, _ = newIndexer(t) + _ = ix.Apply(ctx, pull("novox/mesh-delivery", 12, "feat/x", h1, "modules/a")) + _ = ix.Apply(ctx, merged("novox/mesh-delivery", 12, h1)) + if _, ok := read(t, st, "feat/x"); ok { + t.Error("a lone merged pull request is still a candidate") + } +}