From b0ec4f6f14056757a749c010328521d2b3e3c2bd Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 18 Oct 2026 04:27:42 +0200 Subject: [PATCH] A group member refused for a pending status of its own asks its own check once, not the composed check (issue 1049) --- .../cmd/mesh-delivery/group_test.go | 43 +++++++++++++ .../mesh-delivery/cmd/mesh-delivery/holder.go | 64 ++++++++++++++++--- .../mesh-delivery/cmd/mesh-delivery/ready.go | 3 + 3 files changed, 100 insertions(+), 10 deletions(-) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/group_test.go b/modules/mesh-delivery/cmd/mesh-delivery/group_test.go index 88967366..f508a157 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/group_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/group_test.go @@ -2,6 +2,7 @@ package main import ( "encoding/json" + "errors" "sort" "strings" "testing" @@ -395,3 +396,45 @@ func TestAStaleComposedPassLandingAfterTheMovedMembersFreshVerdictIsAskedAgain(t t.Fatalf("the operator was asked before the new composed check passed: %d", w.op.count()) } } + +// ownPending is the merge tool's refusal of a member whose own mesh/repo-check stands pending after the group's composed +// check set its gate (issue 1049: the forge's "not a success: mesh/repo-check (pending)"). +var ownPending = errors.New("the forge refused gitea_merge_pull_request: novox/mesh-controller#1 is not merged: main requires " + + "mesh/merge-gate, mesh/repo-check; not a success: mesh/repo-check (pending). A pull request merges once its checks succeed") + +// ownChecks are the checks asked from index on that were a head's own (no group), not a group's composed check. +func ownChecks(w *world, from int) (own, composed []string) { + for _, c := range w.ctl.checks[from:] { + if strings.HasPrefix(c, ":") { + own = append(own, c) + } else { + composed = append(composed, c) + } + } + return +} + +// Issue 1049 (the 2026-10-18 02:04 group): a closed group's member refused for a pending status of its OWN (the repository +// check a sibling's new head put back) asks its own check, once for the head, and not the composed check, which sets no +// repository check; a second pass at the same head asks nothing more. +func TestAClosedGroupsMemberRefusedForItsOwnPendingStatusAsksItsOwnCheckOncePerHead(t *testing.T) { + w := approvedGroup(t) + w.forge.refuseRepo = map[string]error{gCtl: ownPending} + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + w.h.PullMerged(merged(gCat, 2, gHk, gT1)) + asked := len(w.ctl.checks) + w.settleAll() + own, composed := ownChecks(w, asked) + if len(composed) != 0 || len(own) != 1 || own[0] != ":"+IDOf(gCtl, gHc) { + t.Fatalf("own checks %v, composed checks %v; wanted the member's own check once and no composed check", own, composed) + } + // Later passes, the same head, the same refusal: nothing more is asked. + w.later(2 * time.Minute) + w.settleAll() + w.later(2 * time.Minute) + w.settleAll() + if got := len(w.ctl.checks) - asked; got != 1 { + t.Fatalf("asked %d checks at the same head, wanted the one: %v", got, w.ctl.checks[asked:]) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index b8fa3fcf..d9a98464 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -1922,11 +1922,24 @@ func (h *Holder) planGroup(g *Group, members []*Delivery, now time.Time) []func( // A closed group's member whose merge the tool refused, after the composed check answered, is judged again at once // (novox/hq ADR 0418 ยง4, issue 1082): the refusal is a sibling's merge having moved what the verdict stands on, and // nothing else asks the composed check again. Not while a check is on its way (its verdict is empty then). + // **A refusal that names the member's OWN pending status** (mesh/repo-check, with the gate not pending: the composed + // check set it) is not that: the composed check sets no repository check, so asking it again changes nothing. The + // member's own check is asked, once for the head (issue 1049); its verdict sets the status. if !again && g.Closed && g.Check != nil && g.Check.Verdict != "" { - if refused := refusedSince(members, g.Check.At); refused != nil { + for _, d := range members { + if !refusedAfter(d, g.Check.At) { + continue + } + if ownStatusPending(d.Ready.Answer) { + if a := h.askOwnAfterRefusal(d); a != nil { + asks = append(asks, a) + } + continue + } again = true h.Logf("[mesh-delivery] group %s: the merge of %s was refused (%s), so its composed check is asked again", g.ID, - refused.ID, refused.Ready.Answer) + d.ID, d.Ready.Answer) + break } } if allReady && (g.Check == nil && !g.Closed || again) { @@ -2507,16 +2520,47 @@ func containsString(xs []string, x string) bool { return false } -// rejudgedSince is whether a member was judged again after a moment: a verdict of its own heard since. -// refusedSince is a member whose merge the tool refused, asked at or after the time given and not merged since: the first in -// the group's order, or nil. -func refusedSince(members []*Delivery, at time.Time) *Delivery { - for _, d := range members { - if r := d.Ready; r != nil && r.Merged == nil && d.MergedAs == "" && r.RefusedSince != nil && !r.LastTry.Before(at) { - return d +// refusedAfter is whether a member's merge, not yet merged, was refused by the merge tool in a try made at or after at. +func refusedAfter(d *Delivery, at time.Time) bool { + r := d.Ready + return r != nil && r.Merged == nil && d.MergedAs == "" && r.RefusedSince != nil && !r.LastTry.Before(at) +} + +// ownStatusPending is whether the merge tool refused a merge because a status of the member's own merge check is +// pending while its gate is not: the forge's "not a success: mesh/repo-check (pending)" (gitea merge.ts, mergeableUnder). +// A refusal that also names mesh/merge-gate is the group's composed check's to answer, as is a verdict out of date. +func ownStatusPending(answer string) bool { + _, listed, ok := strings.Cut(answer, "not a success:") + return ok && strings.Contains(listed, "mesh/repo-check (pending)") && !strings.Contains(listed, "mesh/merge-gate") +} + +// askOwnAfterRefusal asks a member's own check once for its head, after the merge tool refused it for a pending status of +// its own. Under the lock; the ask is made outside it. Nil when it was asked already for this head. +func (h *Holder) askOwnAfterRefusal(d *Delivery) func() { + r := d.Ready + if !r.OwnAskedAt.IsZero() { + return nil + } + r.OwnAskedAt = h.Now() + h.keep(d) + id, m := d.ID, MemberOf(d) + return func() { + asked, err := h.Controller.Check("", []Member{m}) + h.mu.Lock() + defer h.mu.Unlock() + d := h.deliveries[id] + if err != nil { + h.Logf("[mesh-delivery] %s's own check cannot be asked yet: %v", id, err) + if d != nil && d.Ready != nil { + d.Ready.OwnAskedAt = time.Time{} + h.keep(d) + } + h.again("a member's own check that could not be asked") + return } + h.Logf("[mesh-delivery] %s: the merge tool refused it for a pending status of its own, so its own check is asked as %s", + id, asked) } - return nil } func rejudgedSince(members []*Delivery, at time.Time) bool { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ready.go b/modules/mesh-delivery/cmd/mesh-delivery/ready.go index 4c7449d3..1934f4ff 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ready.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ready.go @@ -65,6 +65,9 @@ type ReadyRun struct { Answer string `json:"answer,omitempty"` // merging is whether a merge ask is on its way, outside the lock: not asked twice meanwhile. Not kept. merging bool + // OwnAskedAt is when its own check was asked after the merge tool refused it for a pending status of its own (issue + // 1049): once for the head this run is for. + OwnAskedAt time.Time `json:"own_asked_at,omitempty"` // RefusedSince is when the merge tool began refusing it without a break. RefusedSince *time.Time `json:"refused_since,omitempty"` // Merged is when the merge tool merged it. -- 2.54.0