From 983e4a64448b4838179a99e0ed79cd12da70fe98 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 18 Oct 2026 01:09:37 +0200 Subject: [PATCH] A closed group's member whose merge was refused asks the composed check again at once, and a refusal loop past ten minutes is said (issue 1082) --- .../cmd/mesh-delivery/group_test.go | 66 +++++++++++++++++++ .../mesh-delivery/cmd/mesh-delivery/holder.go | 21 ++++++ .../mesh-delivery/cmd/mesh-delivery/ready.go | 19 ++++++ 3 files changed, 106 insertions(+) diff --git a/modules/mesh-delivery/cmd/mesh-delivery/group_test.go b/modules/mesh-delivery/cmd/mesh-delivery/group_test.go index 405c983f..faf97c2d 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/group_test.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/group_test.go @@ -3,6 +3,7 @@ package main import ( "encoding/json" "sort" + "strings" "testing" "time" ) @@ -219,3 +220,68 @@ func TestAComposedCheckAskedAfterOneMemberMergedIsAskedWithAllMembers(t *testing t.Fatalf("the composed check was asked with %v; wanted once, with both members (the merged one at its head)", composed) } } + +// Issue 1082 (the 2026-10-17 23:48 group): a closed group's member whose merge the tool refused after its sibling merged +// asks the group's composed check again at once, and merges on its pass with the one Approve (ADR 0418 §4, ADR 0421). +func TestAClosedGroupsMemberWhoseMergeWasRefusedAsksTheComposedCheckAgainAtOnce(t *testing.T) { + w := approvedGroup(t) + w.forge.refuseRepo = map[string]error{gCtl: errAskedAgain} + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + // The catalogue member merged, the controller's was refused. + w.h.PullMerged(merged(gCat, 2, gHk, gT1)) + asked := len(w.ctl.checks) + w.settleAll() + var composed []string + for _, c := range w.ctl.checks[asked:] { + if strings.HasPrefix(c, "feat/g:") { + composed = append(composed, c) + } + } + if len(composed) != 1 || composed[0] != "feat/g:"+IDOf(gCat, gHk)+","+IDOf(gCtl, gHc) { + t.Fatalf("the refusal asked the composed check %v; wanted once, at once, with both members", composed) + } + // Not again while that check is on its way. + w.later(2 * time.Minute) + w.settleAll() + if got := w.ctl.checks[asked:]; len(got) != 1 { + t.Fatalf("asked again while the composed check was on its way: %v", got) + } + // It passes; the controller's member merges on the same Approve. + w.forge.refuseRepo = nil + g := w.h.groups["feat/g"] + w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed again"}) + w.settleAll() + if d := w.delivery(IDOf(gCtl, gHc)); d.Ready.Merged == nil { + t.Fatalf("the refused member was not merged on the new composed verdict: %+v", d.Ready) + } + if w.op.count() != 1 { + t.Fatalf("a second Approve was asked: %d", w.op.count()) + } +} + +// Issue 1082: a refusal loop is not silent. A merge refused past ten minutes in a closed group (ADR 0418 §3) is a stalled +// line naming the member and the merge tool's last answer, which the controller raises as a condition. +func TestARefusalLoopInAClosedGroupIsSaidPastTenMinutes(t *testing.T) { + w := approvedGroup(t) + w.forge.refuseRepo = map[string]error{gCtl: errAskedAgain} + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + w.h.PullMerged(merged(gCat, 2, gHk, gT1)) + w.settleAll() + // Each composed pass is followed by a refusal again: the loop. + for range 12 { + w.later(time.Minute) + g := w.h.groups["feat/g"] + w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed again"}) + w.settleAll() + if lines := w.h.Stalled(); len(lines) != 0 && w.now.Sub(*w.delivery(IDOf(gCtl, gHc)).Ready.RefusedSince) < 10*time.Minute { + t.Fatalf("said before its bound: %+v", lines) + } + } + lines := w.h.Stalled() + if len(lines) != 1 || lines[0].ID != IDOf(gCtl, gHc) || lines[0].Bound != "10m0s" || + !strings.Contains(lines[0].Says, errAskedAgain.Error()) || !strings.Contains(lines[0].Says, "group feat/g") { + t.Fatalf("the loop was not said, naming the member, its group and the tool's answer: %+v", lines) + } +} diff --git a/modules/mesh-delivery/cmd/mesh-delivery/holder.go b/modules/mesh-delivery/cmd/mesh-delivery/holder.go index 8b2ea3a1..a91fbee6 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/holder.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/holder.go @@ -1847,6 +1847,16 @@ func (h *Holder) planGroup(g *Group, members []*Delivery, now time.Time) []func( } } again := g.Check != nil && (!sameMembers(g.Check.Commits, commits) || g.Check.OutOfDate != "" && rejudgedSince(members, g.Check.OutOfDateAt)) + // 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). + if !again && g.Closed && g.Check != nil && g.Check.Verdict != "" { + if refused := refusedSince(members, g.Check.At); refused != nil { + 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) + } + } if allReady && (g.Check == nil && !g.Closed || again) { ms := membersOfAsk(members) id := g.ID @@ -2426,6 +2436,17 @@ func containsString(xs []string, x string) bool { } // 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 + } + } + return nil +} + func rejudgedSince(members []*Delivery, at time.Time) bool { for _, d := range members { if d.Check != nil && d.Check.At.After(at) { diff --git a/modules/mesh-delivery/cmd/mesh-delivery/ready.go b/modules/mesh-delivery/cmd/mesh-delivery/ready.go index 19d92742..fb2de9f7 100644 --- a/modules/mesh-delivery/cmd/mesh-delivery/ready.go +++ b/modules/mesh-delivery/cmd/mesh-delivery/ready.go @@ -41,6 +41,16 @@ const mergeRetryAfter = time.Minute // check takes (the controller's checkTimeout is 45 minutes), so a check on its way is never said. const readyRefusedFor = time.Hour +// groupRefusedFor is how long the merge tool may refuse a closed group's member before it is said as stalled: the merge +// window of ADR 0418 §3, not the hour of a lone head, because the group's other members are already on the trunk. +const groupRefusedFor = 10 * time.Minute + +// groupClosed is whether a group is closed: a member reached the trunk. Under the lock. +func (h *Holder) groupClosed(id string) bool { + g := h.groups[id] + return g != nil && g.Closed +} + // ReadyRun is what this owner did with a delivery's Approve and merge: kept on the delivery and shown whole by `show`. type ReadyRun struct { // Head is the head the run is for. @@ -407,6 +417,15 @@ func (h *Holder) readyLines(now time.Time) []StalledLine { Bound: "0s", H2: "none: a new head, which asks the operator again — the operator's", Says: fmt.Sprintf("%s#%d at %s is not merged: the operator's answer %s", d.Repository, d.Number, shortOf(r.Head, 12), r.Refused)}) + case r.RefusedSince != nil && d.Group != "" && h.groupClosed(d.Group) && now.Sub(*r.RefusedSince) >= groupRefusedFor: + // A closed group's member (novox/hq ADR 0418 §3, issue 1082): its sibling merged, and its own merge is refused + // again after each composed check. Said after the merge window, with the group and the tool's last answer. + out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State, + For: now.Sub(*r.RefusedSince).Round(time.Second).String(), Bound: groupRefusedFor.String(), + H2: "none: the group's composed check is asked again after each refusal, and the merge each minute — the operator's", + Says: fmt.Sprintf("%s#%d is approved at %s with its group %s closed (a sibling merged), and the merge tool has "+ + "refused it since %s: %s", d.Repository, d.Number, shortOf(r.Head, 12), d.Group, + r.RefusedSince.UTC().Format(time.RFC3339), r.Answer)}) case r.RefusedSince != nil && now.Sub(*r.RefusedSince) >= readyRefusedFor: out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State, For: now.Sub(*r.RefusedSince).Round(time.Second).String(), Bound: readyRefusedFor.String(),