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)

This commit is contained in:
2026-10-18 01:09:37 +02:00
parent da094008ea
commit 983e4a6444
3 changed files with 106 additions and 0 deletions
@@ -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)
}
}
@@ -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) {
@@ -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(),