diff --git a/modules/mesh-issues/cmd/mesh-issues/delivered_test.go b/modules/mesh-issues/cmd/mesh-issues/delivered_test.go index 2d6e3584..55fb7c27 100644 --- a/modules/mesh-issues/cmd/mesh-issues/delivered_test.go +++ b/modules/mesh-issues/cmd/mesh-issues/delivered_test.go @@ -94,3 +94,40 @@ func TestTheSameRevisionTwiceIsOnceAndTheGatesOwnDeliveredIsNoWord(t *testing.T) t.Fatalf("delivered %v, %d ready-to-verify", is.Delivered, countEvents(h, EventReadyToVerify)) } } + +// "A newer revision covers older ones": the view says each revision once and may skip one delivered together with a later one, +// so a merged fix whose merge is an ancestor of the head the event holds for its repository is delivered too, as the forge says. +func TestANewerRevisionCoversTheOlderMergesOfItsRepositoryAsTheForgeSaysAncestry(t *testing.T) { + h := newHarness(t) + older := fixIssue(t, h, "mesh-catalog#30") + newer := fixIssue(t, h, "mesh-catalog#31") + unrelated := fixIssue(t, h, "mesh-catalog#32") + mergedFix(t, h, 30, "feat/a", "1111111111110000") + mergedFix(t, h, 31, "feat/b", "2222222222220000") + mergedFix(t, h, 32, "feat/c", "3333333333330000") + var asked []string + h.Contains = func(owner, repo, commit, in string) (bool, error) { + asked = append(asked, owner+"/"+repo+" "+commit+" in "+in) + return commit == "1111111111110000", nil // only the first is an ancestor of the revision's head + } + viewSays(t, h, 12, "feat/b", map[string]string{"novox/mesh-catalog": "2222222222220000"}) + for n, want := range map[uint64]string{older: "delivered to every machine it should reach", + newer: "delivered to every machine it should reach", unrelated: "fix merged, not delivered: mesh-catalog#32"} { + if v, _ := h.View(n); v.Delivery != want { + t.Errorf("issue %d reads %q, wanted %q (asked %v)", n, v.Delivery, want, asked) + } + } + // A forge that cannot say leaves the fix for the next event, and a repository the revision holds no head for is not asked. + h2 := newHarness(t) + n := fixIssue(t, h2, "mesh-catalog#33") + mergedFix(t, h2, 33, "feat/d", "4444444444440000") + h2.Contains = func(string, string, string, string) (bool, error) { return false, errStuck } + viewSays(t, h2, 13, "feat/z", map[string]string{"novox/mesh-catalog": "5555555555550000"}) + if v, _ := h2.View(n); v.Delivery != "fix merged, not delivered: mesh-catalog#33" { + t.Fatalf("a forge that could not say delivered it: %q", v.Delivery) + } + viewSays(t, h2, 14, "feat/z", map[string]string{"novox/mesh-sdk": "5555555555550000"}) + if v, _ := h2.View(n); v.Delivery != "fix merged, not delivered: mesh-catalog#33" { + t.Fatalf("another repository's head covered it: %q", v.Delivery) + } +} diff --git a/modules/mesh-issues/cmd/mesh-issues/delivery.go b/modules/mesh-issues/cmd/mesh-issues/delivery.go index b23e023c..01835b5f 100644 --- a/modules/mesh-issues/cmd/mesh-issues/delivery.go +++ b/modules/mesh-issues/cmd/mesh-issues/delivery.go @@ -427,9 +427,9 @@ const DeliveredEvent = "mesh-delivery-view.delivered" // OnDelivered is the handler of the view's `delivered` event: {revision, group, heads}. Every merged fix of a pull request the // group moved, or whose merge is the head the revision holds for its repository, reached the machines at that revision. The same -// revision twice changes nothing (a delivered fix stays delivered), and a fix is matched by the group it merged in or by its own -// merge commit, so a revision that carries a later head of a repository does not cover an earlier merge the tracker never heard -// delivered: that waits for its own revision's event. +// revision twice changes nothing (a delivered fix stays delivered). A fix is matched by the group it merged in, by its own merge +// commit, or because its merge is an ancestor of the head the revision holds for its repository (coveredBy): a newer revision +// covers the older ones, which the view may never have said. func (t *Tracker) OnDelivered(e stdio.Envelope) error { var body struct { Revision uint64 `json:"revision"` @@ -453,7 +453,7 @@ func (t *Tracker) OnDelivered(e stdio.Envelope) error { for n, is := range all { for _, pr := range liveFixes(is) { f := fixOf(is, pr) - if f == nil || !f.Merged || contains(is.Delivered, pr) || !deliveredBy(f, body.Group, body.Heads) { + if f == nil || !f.Merged || contains(is.Delivered, pr) || !(deliveredBy(f, body.Group, body.Heads) || t.coveredBy(f, body.Heads)) { continue } pr := pr @@ -482,6 +482,57 @@ func (t *Tracker) OnDelivered(e stdio.Envelope) error { return nil } +// busContains asks the forge's gitea_contains over the bus: whether commit is an ancestor of in, or it. +func busContains(owner, repo, commit, in string) (bool, error) { + raw, err := stdio.Ask("gitea.gitea_contains", map[string]any{"owner": owner, "repo": repo, "commit": commit, "in": in}) + if err != nil { + return false, err + } + var out struct { + Contains *bool `json:"contains"` + Answer struct { + Contains *bool `json:"contains"` + } `json:"answer"` + } + if json.Unmarshal(raw, &out) != nil { + return false, errors.New("the forge's gitea_contains is not readable") + } + switch { + case out.Answer.Contains != nil: + return *out.Answer.Contains, nil + case out.Contains != nil: + return *out.Contains, nil + } + return false, errors.New("the forge's gitea_contains answered no verdict") +} + +// coveredBy says whether a merged fix's merge is an ancestor of, or equal to, the head a delivered revision holds for its +// repository: the view says each revision once and may skip one delivered together with a later one, so a newer revision +// covers the older merges of its repository. The forge decides ancestry; a forge that cannot say leaves the fix for the next event. +func (t *Tracker) coveredBy(f *Fix, heads map[string]string) bool { + if t.Contains == nil || f.MergedAs == "" { + return false + } + for repository, head := range heads { + if !strings.EqualFold(repository, f.Repository) { + continue + } + owner, repo, ok := strings.Cut(repository, "/") + if !ok { + continue + } + yes, err := t.Contains(owner, repo, f.MergedAs, head) + if err != nil { + t.Logf("[mesh-issues] whether %s contains %s is not known yet: %v", shortCommit(head), shortCommit(f.MergedAs), err) + continue + } + return yes + } + return false +} + +func shortCommit(c string) string { return c[:min(len(c), 12)] } + // deliveredBy says whether a merged fix is one a delivered revision names: the group it merged in is the group that moved the // source, or its merge commit is the head the revision holds for its repository. func deliveredBy(f *Fix, group string, heads map[string]string) bool { diff --git a/modules/mesh-issues/cmd/mesh-issues/main.go b/modules/mesh-issues/cmd/mesh-issues/main.go index ca2c33c4..742f1503 100644 --- a/modules/mesh-issues/cmd/mesh-issues/main.go +++ b/modules/mesh-issues/cmd/mesh-issues/main.go @@ -105,6 +105,7 @@ func main() { return busDeliveryState(forgeOwner())(repository, number) } t.Approval = busApproval(forgeOwner) + t.Contains = busContains if len(os.Args) > 1 && os.Args[1] == "dry-run" { os.Exit(dryRunAtTheTerminal(t, os.Args[2:])) } diff --git a/modules/mesh-issues/cmd/mesh-issues/manifest_test.go b/modules/mesh-issues/cmd/mesh-issues/manifest_test.go index db7df92b..68b60afa 100644 --- a/modules/mesh-issues/cmd/mesh-issues/manifest_test.go +++ b/modules/mesh-issues/cmd/mesh-issues/manifest_test.go @@ -112,7 +112,7 @@ func TestItDeclaresAndHoldsTheIssueTrackerSeatOnceForTheMesh(t *testing.T) { // the import and a reservation read the record through the records module (issue 424). if !reflect.DeepEqual(m.Consumes, []string{"mesh-issues." + EventOpened, "mesh-issues." + EventMoved, "mesh-delivery-gate.transition", DeliveredEvent}) || !reflect.DeepEqual(m.Invokes, []string{"claude-code.claude_code_judge", "seat:mesh-delivery.checks", "seat:operator-channel.notify", - "gitea.gitea_pull_reviews", "records.records_status", "records.records_list", "records.records_read", "mesh-knowledge.knowledge_highest"}) { + "gitea.gitea_pull_reviews", "gitea.gitea_contains", "records.records_status", "records.records_list", "records.records_read", "mesh-knowledge.knowledge_highest"}) { t.Fatalf("consumes %v, invokes %v", m.Consumes, m.Invokes) } // The tools bundle, and the archive the build packs files/ into for the templates (novox/hq ADR 0349 §2). diff --git a/modules/mesh-issues/cmd/mesh-issues/tracker.go b/modules/mesh-issues/cmd/mesh-issues/tracker.go index 327506f7..5bea5f3f 100644 --- a/modules/mesh-issues/cmd/mesh-issues/tracker.go +++ b/modules/mesh-issues/cmd/mesh-issues/tracker.go @@ -36,6 +36,9 @@ type Tracker struct { Judge Judgement // Deliveries answers a pull request's delivery (ADR 0279 §3, ADR 0281); nil asks nothing. Deliveries DeliveryState + // Contains asks the forge whether a commit is contained in another (gitea_contains); nil asks nothing, and a delivered + // revision then marks only the fixes it names (OnDelivered). + Contains func(owner, repo, commit, in string) (bool, error) // Notify tells the operator's channel of a stuck fix (ADR 0281) and of a fix over its budget (ADR 0313); nil // says nothing there. Notify Notifier diff --git a/modules/mesh-issues/module.json b/modules/mesh-issues/module.json index e144bbf0..ee14150a 100644 --- a/modules/mesh-issues/module.json +++ b/modules/mesh-issues/module.json @@ -65,6 +65,7 @@ "seat:mesh-delivery.checks", "seat:operator-channel.notify", "gitea.gitea_pull_reviews", + "gitea.gitea_contains", "records.records_status", "records.records_list", "records.records_read",