Merge a group whole or not at all, and say loudly when it cannot (issues 1275, 1276) #375

Merged
mesh-tools merged 3 commits from fix/1275-group-merges-whole-or-not into main 2026-10-19 18:45:31 +00:00
11 changed files with 689 additions and 70 deletions
@@ -318,24 +318,74 @@ func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) {
id := fmt.Sprintf("merge-%d-%s-%d", d.Number, shortOf(d.Commit, 12), r.Asks)
ask := composeMergeAsk(id, d, group, members, heads, now)
taken := []*Delivery{}
var reads []memberRead
for _, m := range members {
h.runOf(m, now)
m.Ready.AskID, m.Ready.Ask, m.Ready.Heads, m.Ready.AskedAt, m.Ready.Asks = id, &ask, heads, now, max(m.Ready.Asks, r.Asks)
h.keep(m)
taken = append(taken, m)
reads = append(reads, readOf(m, m.Commit))
}
h.Logf("[mesh-delivery] %s passed its checks: the operator is asked to Approve its merge into main (%s, ask %d)",
d.ID, id, r.Asks)
gid := ""
if group != "" {
gid = d.Group
}
return "the operator's Approve is asked (" + id + ")", func() {
if err := h.Operator.Ask(ask); err != nil {
h.mu.Lock()
defer h.mu.Unlock()
// A group is not asked while a member's required status is not a success at its head (issue 1275): the group
// says which, and is asked once every member may merge.
var holdMember, holdSays string
var readErr error
for _, m := range reads {
if gid == "" {
break
}
says, _, err := h.notASuccess(m)
if err != nil {
readErr = err
break
}
if says != "" {
holdMember, holdSays = m.id, says
break
}
}
undo := func() {
for _, m := range taken {
if m.Ready != nil && m.Ready.AskID == id {
m.Ready.AskID, m.Ready.Ask, m.Ready.Heads = "", nil, nil
h.keep(m)
}
}
}
if readErr != nil || holdSays != "" {
h.mu.Lock()
defer h.mu.Unlock()
undo()
if g := h.groups[gid]; g != nil && holdSays != "" {
if g.Hold == nil || g.Hold.Kind != "unasked" || g.Hold.Says != holdSays || g.Hold.Member != holdMember {
h.Logf("[mesh-delivery] group %s is not asked for its Approve: %s waits on a required status: %s", gid, holdMember, holdSays)
g.Hold = &GroupHold{Kind: "unasked", Member: holdMember, Says: holdSays, Since: h.Now()}
h.keepGroup(g)
}
}
if readErr != nil {
h.Logf("[mesh-delivery] group %s: the required statuses cannot be read, so the operator is not asked yet: %v", gid, readErr)
}
h.again("a group's Approve held for a required status")
return
}
if g := func() *Group { h.mu.Lock(); defer h.mu.Unlock(); return h.groups[gid] }(); g != nil && g.Hold != nil && g.Hold.Kind == "unasked" {
h.mu.Lock()
g.Hold = nil
h.keepGroup(g)
h.mu.Unlock()
}
if err := h.Operator.Ask(ask); err != nil {
h.mu.Lock()
defer h.mu.Unlock()
undo()
h.again("the operator's Approve, which could not be asked")
h.Logf("[mesh-delivery] the operator could not be asked about %s (%v); asked again in %s", d.ID, err, askAgainAfter)
}
@@ -306,6 +306,17 @@ func (r *runtime) answer(m rpcMessage) {
return toolAnswer(map[string]any{"asked": "check-1"}), nil
case "gitea.gitea_open_pulls":
return toolAnswer(map[string]any{"pulls": pulls}), nil
case "gitea.gitea_commit_statuses":
// Green: both merge checks a success at the head asked, as the gitea module answers.
sha, _ := a.Body["sha"].(string)
if sha == "" {
sha = "0000000"
}
return toolAnswer(map[string]any{"commit": sha, "statuses": []any{
map[string]any{"context": "mesh/merge-gate", "state": "success"},
map[string]any{"context": "mesh/repo-check", "state": "success"}},
"merge": map[string]any{"branch": "main", "protected": true, "required": []string{"mesh/merge-gate", "mesh/repo-check"},
"blocking": []any{}, "mergeable": true, "says": "main requires mesh/merge-gate, mesh/repo-check"}}), nil
case "gitea.gitea_note_append", "gitea.gitea_delivery_view", "gitea.gitea_commit_status", "gitea.gitea_merge_pull_request",
"gitea.gitea_add_comment":
return toolAnswer(map[string]any{}), nil
@@ -1,6 +1,7 @@
package main
import (
"fmt"
"slices"
"strings"
@@ -63,11 +64,12 @@ func (h *Holder) CommitStatus(e ForgeStatusEvent) {
return
}
h.wantForge()
if e.State != "success" {
return
}
h.mu.Lock()
defer h.mu.Unlock()
if e.State != "success" {
h.withdrawForStatus(e)
return
}
repository := strings.ToLower(e.Owner + "/" + e.Repo)
for _, d := range h.deliveries {
if !strings.EqualFold(d.Repository, repository) || d.Ready == nil || d.Ready.Merged != nil || d.MergedAs != "" ||
@@ -83,3 +85,26 @@ func (h *Holder) CommitStatus(e ForgeStatusEvent) {
}
}
}
// withdrawForStatus takes back the ask of an Approve nobody answered yet when a merge check of a head it covers is no
// longer a success (issue 1275, AC5): the answer to a withdrawn ask merges nothing, and the operator is asked again once
// every member may merge. Under the lock.
func (h *Holder) withdrawForStatus(e ForgeStatusEvent) {
repository := strings.ToLower(e.Owner + "/" + e.Repo)
done := map[string]bool{}
for _, d := range h.deliveries {
r := d.Ready
if !strings.EqualFold(d.Repository, repository) || r == nil || r.AskID == "" || r.Approved != nil || r.Declined != nil || done[r.AskID] {
continue
}
head := d.Commit
if sha := r.Heads[d.ID]; sha != "" {
head = sha
}
if sameCommit(head, e.SHA) {
done[r.AskID] = true
h.cancels = append(h.cancels, h.withdrawAsk(r.AskID, fmt.Sprintf("%s of %s is %s at its head %s", e.Context, d.ID, e.State, shortOf(head, 12))))
h.Poke()
}
}
}
@@ -504,17 +504,17 @@ func (f *fakeForge) Statuses(owner, repo string, number int, commit, base string
f.asked = append(f.asked, fmt.Sprintf("%s/%s#%d@%s base=%s", owner, repo, number, commit, base))
out := &ForgeChecks{}
if number > 0 {
p, ok := f.pulls[fmt.Sprintf("%s/%s#%d", owner, repo, number)]
if !ok {
if p, ok := f.pulls[fmt.Sprintf("%s/%s#%d", owner, repo, number)]; ok {
out.Pull = &p
if commit == "" {
commit = p.HeadSHA
}
if base == "" {
base = p.Base
}
} else if commit == "" {
return nil, fmt.Errorf("no pull request %d", number)
}
out.Pull = &p
if commit == "" {
commit = p.HeadSHA
}
if base == "" {
base = p.Base
}
}
for key, said := range f.statuses {
at, context, _ := strings.Cut(key, " ")
@@ -0,0 +1,229 @@
package main
import (
"fmt"
"strings"
"time"
)
// A group is merged whole or not at all, in its order (issue 1275, novox/hq ADR 0418).
//
// - Before the operator is asked, and again before any member is merged, every member's required statuses are read at
// its head on the forge; one that is not a success holds the group, and nothing is merged.
// - The members are merged one after the other in the group's order, and the pass stops at the first refusal: no later
// member is merged.
// - A group held or half merged is said as a stalled line, urgent, until every member is merged or a person decides.
// memberRead is what a status read of one member needs, taken under the lock.
type memberRead struct {
id, owner, repo, base, head string
number int
}
// readOf is a delivery as a status read needs it, at the head the Approve or the ask covers.
func readOf(d *Delivery, head string) memberRead {
if head == "" {
head = d.Commit
}
return memberRead{id: d.ID, owner: d.Owner(), repo: d.Repo(), base: d.Base, head: head, number: d.Number}
}
// notAsSuccess reads a member's required statuses at its head and says, in the forge's words, which are not a success;
// empty when all are. ownOnly is whether only the member's own repository check is missing or pending. An error is a
// read that could not be made: the member then counts as not mergeable.
func (h *Holder) notASuccess(m memberRead) (says string, ownOnly bool, err error) {
fc, err := h.Forge.Statuses(m.owner, m.repo, m.number, m.head, m.base)
if err != nil {
return "", false, err
}
if fc.Merge.Mergeable == nil {
return "", false, fmt.Errorf("the forge could not say whether %s may merge: %s", m.id, fc.Merge.Says)
}
if *fc.Merge.Mergeable {
return "", false, nil
}
var parts []string
ownOnly = len(fc.Merge.Blocking) > 0
for _, b := range fc.Merge.Blocking {
parts = append(parts, fmt.Sprintf("%s (%s)", b.Context, b.State))
if b.Context != ctxRepo || (b.State != "pending" && b.State != "missing") {
ownOnly = false
}
}
return fmt.Sprintf("%s requires %s; not a success: %s", fc.Merge.Branch, strings.Join(fc.Merge.Required, ", "),
strings.Join(parts, ", ")), ownOnly, nil
}
// unmergedLive are a group's members not yet merged and not over, in the group's order. Under the lock.
func (h *Holder) unmergedLive(g *Group) []*Delivery {
var out []*Delivery
for _, d := range h.membersOf(g) {
if d.MergedAs == "" && !d.State.Final() && (d.Ready == nil || d.Ready.Merged == nil) {
out = append(out, d)
}
}
return out
}
// groupPass is the one pass that merges a group's members, made by the first of them in the group's order that may be
// merged; nil when it is not d's turn, or a member is not approved at its head. Under the lock; the pass runs outside it.
func (h *Holder) groupPass(g *Group, d *Delivery, now time.Time) func() {
order := h.unmergedLive(g)
if len(order) == 0 || order[0] != d {
return nil
}
var pass []passMember
for _, m := range order {
c := h.covering(m)
if m.Ready == nil || m.Ready.Declined != nil || c == nil || c.Approved == nil || c.Heads[m.ID] != m.Commit || h.readyWaits(m) != "" {
return nil // the others are not approved at their heads yet: nothing is merged before they are
}
pass = append(pass, passMember{read: readOf(m, c.Heads[m.ID]), heads: c.Heads})
}
for _, m := range order {
m.Ready.Tries++
m.Ready.LastTry, m.Ready.merging = now, true
h.keep(m)
}
gid := g.ID
return func() { h.mergeGroupPass(gid, pass) }
}
// passMember is one member of a pass as it was taken under the lock.
type passMember struct {
read memberRead
heads map[string]string
}
// mergeGroupPass reads every member's required statuses, and only when all are a success merges the members in order,
// stopping at the first the merge tool refuses.
func (h *Holder) mergeGroupPass(gid string, pass []passMember) {
release := func(from int, retry bool) {
for _, p := range pass[from:] {
if d := h.deliveries[p.read.id]; d != nil && d.Ready != nil {
d.Ready.merging = false
if retry {
d.Ready.LastTry = time.Time{}
}
}
}
}
for _, p := range pass {
says, ownOnly, err := h.notASuccess(p.read)
if err == nil && says == "" {
continue
}
h.mu.Lock()
if err != nil {
h.Logf("[mesh-delivery] group %s: the required statuses of %s cannot be read, so nothing is merged yet: %v", gid, p.read.id, err)
release(0, true)
h.again("a group's merge whose required statuses could not be read")
h.mu.Unlock()
return
}
var own func()
if g := h.groups[gid]; g != nil {
if g.Hold == nil || g.Hold.Kind != "blocked" || g.Hold.Says != says || g.Hold.Member != p.read.id {
h.Logf("[mesh-delivery] group %s is approved and NOT merged: %s at %s: %s", gid, p.read.id, shortOf(p.read.head, 12), says)
}
since := time.Time{}
if g.Hold != nil && g.Hold.Kind == "blocked" {
since = g.Hold.Since
}
if since.IsZero() {
since = h.Now()
}
g.Hold = &GroupHold{Kind: "blocked", Member: p.read.id, Says: says, Since: since}
h.keepGroup(g)
}
if d := h.deliveries[p.read.id]; ownOnly && d != nil && d.Ready != nil {
own = h.askOwnAfterRefusal(d) // its own repository check is what is missing: asked, once for the head
}
release(0, true)
h.again("a group's merge held for a required status")
h.mu.Unlock()
if own != nil {
own()
}
return
}
h.mu.Lock()
if g := h.groups[gid]; g != nil && g.Hold != nil {
g.Hold = nil
h.keepGroup(g)
}
h.mu.Unlock()
for i, p := range pass {
if h.mergeOne(p.read.id, p.heads) {
continue
}
h.mu.Lock()
release(i+1, false)
if g := h.groups[gid]; g != nil {
h.Logf("[mesh-delivery] group %s: the merge of %s was refused, so none of the members after it is merged", gid, p.read.id)
h.keepGroup(g)
}
h.mu.Unlock()
return
}
}
// groupLines are the groups that are held or half merged, as urgent stalled lines the controller raises as conditions:
// one for each, on its first member not merged, until every member is merged or a person decides (issue 1275). A member
// the merge tool refused is said by readyLines, with its group; this adds a group held for a required status, and a
// group half merged with no refusal. Under the lock.
func (h *Holder) groupLines(now time.Time) []StalledLine {
var out []StalledLine
for _, g := range h.groups {
live := h.unmergedLive(g)
if len(live) == 0 {
continue
}
first, refused, running, merged := live[0], false, false, 0
for _, d := range live {
if d.Ready != nil && (d.Ready.RefusedSince != nil || d.Ready.Failed != "" || d.Ready.Refused != "") {
refused = true
}
if d.Ready != nil && d.Ready.merging {
running = true
}
}
for _, d := range h.membersOf(g) {
if d.MergedAs != "" || (d.Ready != nil && d.Ready.Merged != nil) {
merged++
}
}
heldOn := h.deliveries[holdMember(g)]
switch {
// A held member the merge tool refused is said by readyLines already: one line for an id, not two.
case g.Hold != nil && g.Hold.Kind == "blocked" && !running && !(heldOn != nil && heldOn.Ready != nil && heldOn.Ready.RefusedSince != nil):
out = append(out, StalledLine{ID: g.Hold.Member, Number: numberOf(h.deliveries[g.Hold.Member]), State: first.State,
For: now.Sub(g.Hold.Since).Round(time.Second).String(), Bound: "0s", Severity: "urgent",
H2: "none: the required status turns success, which asks the merge again — or the operator's decision",
Says: fmt.Sprintf("group %s is approved and nothing is merged (merged %d of %d): %s waits on a required status "+
"at its head: %s. Needed: that status turns success (its check asked again), or the operator decides.",
g.ID, merged, len(h.membersOf(g)), g.Hold.Member, g.Hold.Says)})
case merged > 0 && !refused && !running:
out = append(out, StalledLine{ID: first.ID, Number: first.Number, State: first.State,
For: now.Sub(g.Updated).Round(time.Second).String(), Bound: "0s", Severity: "urgent",
H2: "none: every member merged, or the operator decides",
Says: fmt.Sprintf("group %s is half merged (%d of %d members merged): %s is not merged. Needed: it merges "+
"(its Approve and checks hold at its head), or the operator decides.", g.ID, merged, len(h.membersOf(g)), first.ID)})
}
}
return out
}
func numberOf(d *Delivery) int {
if d == nil {
return 0
}
return d.Number
}
func holdMember(g *Group) string {
if g.Hold == nil {
return ""
}
return g.Hold.Member
}
@@ -89,25 +89,26 @@ func TestAnApprovedGroupWhoseTrunkMovedIsCheckedAgainAndMergedOnTheSameApprove(t
}
}
// ADR 0418 §4: a member the forge refuses is merged again on its fresh check, while the other merged in the same pass.
func TestAMemberTheForgeRefusesIsMergedAgainWhileTheOtherMergedInTheSamePass(t *testing.T) {
// ADR 0418 §4, issue 1275/AC3: a member the forge refuses stops the pass (the members after it are not asked), and is
// merged again on its fresh check, the others after it in the same pass.
func TestAMemberTheForgeRefusesStopsThePassAndIsMergedAgainOnItsFreshCheck(t *testing.T) {
w := approvedGroup(t)
w.forge.refuse = errAskedAgain
w.op.answer(w.op.open()[0], OptionApprove)
w.settleAll()
if len(w.forge.merges) != 2 {
t.Fatalf("both members are asked in the same pass: %v", w.forge.merges)
if len(w.forge.merges) != 1 {
t.Fatalf("the pass stops at the first refusal: %v", w.forge.merges)
}
w.forge.refuse = nil
w.later(2 * time.Minute)
w.settleAll()
if len(w.forge.merges) != 2 {
if len(w.forge.merges) != 1 {
t.Fatalf("a refused member is asked again with no event, on a timer or at each look: %v", w.forge.merges)
}
w.h.Checked(CheckedEvent{Group: "feat/g", ID: w.h.groups["feat/g"].Check.Asked, Verdict: "pass", Summary: "composed again"})
w.settleAll()
if len(w.forge.merges) != 4 {
t.Fatalf("a refused member is asked again on the next composed verdict: %v", w.forge.merges)
if len(w.forge.merges) != 3 {
t.Fatalf("a refused member is asked again on the next composed verdict, then the next: %v", w.forge.merges)
}
if d := w.delivery(IDOf(gCat, gHk)); d.Ready.Merged == nil {
t.Fatalf("not merged: %+v", d.Ready)
@@ -2670,11 +2670,12 @@ func refusedAfter(d *Delivery, at time.Time) bool {
}
// 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).
// pending or missing (issue 1276) 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")
return ok && (strings.Contains(listed, "mesh/repo-check (pending)") || strings.Contains(listed, "mesh/repo-check (missing)")) &&
!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
@@ -0,0 +1,250 @@
package main
import (
"errors"
"strings"
"testing"
"time"
)
const time30s = 31 * time.Second
// Issue 1275 (the 2026-10-19 18:05 group): a group is merged whole or not at all, in its order, and a group that is held or
// half merged is said loudly. Issue 1276/AC1: a refusal naming the member's own repository check as missing asks that check.
const (
gate, repoCheck = "mesh/merge-gate", "mesh/repo-check"
)
// statusOf sets a member head's two merge checks on the forge, and makes both required.
func setChecks(w *world, repository, sha, gateState, repoState string) {
w.forge.required = []string{gate, repoCheck}
for ctx, state := range map[string]string{gate: gateState, repoCheck: repoState} {
key := repository + "@" + sha + " " + ctx
if state == "" {
delete(w.forge.statuses, key) // no status at all: the forge says it is missing
continue
}
w.forge.statuses[key] = state + " said → x"
}
}
func allGreen(w *world) {
setChecks(w, gCat, gHk, "success", "success")
setChecks(w, gCtl, gHc, "success", "success")
}
func refusedWords(repository string) error {
return errors.New("the forge refused gitea_merge_pull_request: " + repository + " is not merged: main requires " +
"mesh/merge-gate, mesh/repo-check; not a success: mesh/repo-check (missing). A pull request merges once its checks succeed")
}
func urgentLines(w *world) []StalledLine {
var out []StalledLine
for _, l := range w.h.Stalled() {
if l.Severity == "urgent" {
out = append(out, l)
}
}
return out
}
// 1275/AC2: an approved group whose member has a required status that is not a success at its head when the merge starts
// is merged not at all, though the first member in order is fine; the group says which member and which status.
func TestAnApprovedGroupWithAMemberWhoseRequiredStatusIsNotASuccessMergesNothing(t *testing.T) {
w := approvedGroup(t)
allGreen(w)
w.op.answer(w.op.open()[0], OptionApprove)
setChecks(w, gCtl, gHc, "success", "") // the second member's repository check went missing after the ask
w.settleAll()
if len(w.forge.merges) != 0 {
t.Fatalf("merged with a required status missing: %v", w.forge.merges)
}
lines := urgentLines(w)
if len(lines) != 1 || !strings.Contains(lines[0].Says, "feat/g") || !strings.Contains(lines[0].Says, IDOf(gCtl, gHc)) ||
!strings.Contains(lines[0].Says, "mesh/repo-check (missing)") || !strings.Contains(lines[0].Says, "Needed") {
t.Fatalf("the held group is not said as one urgent line naming group, member, status and need: %+v", lines)
}
// The status turns success: the whole group merges.
setChecks(w, gCtl, gHc, "success", "success")
w.h.CommitStatus(ForgeStatusEvent{Owner: "novox", Repo: "mesh-controller", SHA: gHc, Context: repoCheck, State: "success"})
w.later(time30s)
w.settleAll()
if len(w.forge.merges) != 2 || len(urgentLines(w)) != 0 {
t.Fatalf("merges %v, lines %+v; wanted both merged and no line", w.forge.merges, urgentLines(w))
}
}
// 1275/AC2 with 1276/AC1: a member whose own repository check is the missing status has that check asked, once.
func TestAHeldGroupWhoseMemberLacksItsOwnRepositoryCheckAsksThatCheck(t *testing.T) {
w := approvedGroup(t)
allGreen(w)
w.op.answer(w.op.open()[0], OptionApprove)
setChecks(w, gCtl, gHc, "success", "")
asked := len(w.ctl.checks)
w.settleAll()
w.later(time30s)
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 %v; wanted the member's own check once", own, composed)
}
}
// 1275/AC3: the merge tool refuses the first member in order; the second is not merged, and the group is said urgent,
// naming the group, the refused member and the forge's words.
func TestARefusedFirstMemberStopsTheGroupAndIsSaidUrgent(t *testing.T) {
w := approvedGroup(t)
allGreen(w)
w.forge.refuseRepo = map[string]error{gCat: refusedWords("novox/mesh-catalog#2")}
w.op.answer(w.op.open()[0], OptionApprove)
w.settleAll()
if len(w.forge.merges) != 1 || !strings.Contains(w.forge.merges[0], "mesh-catalog#2") {
t.Fatalf("wanted only the first member asked: %v", w.forge.merges)
}
lines := urgentLines(w)
if len(lines) != 1 || lines[0].ID != IDOf(gCat, gHk) || !strings.Contains(lines[0].Says, "feat/g") ||
!strings.Contains(lines[0].Says, "mesh/repo-check (missing)") || !strings.Contains(lines[0].Says, "Needed") {
t.Fatalf("not said as one urgent line: %+v", lines)
}
}
// 1275/AC4: a group half merged (the second member refused after the first merged) stays raised, pass after pass, until
// the second is merged; then it goes.
func TestAHalfMergedGroupStaysRaisedUntilEveryMemberIsMerged(t *testing.T) {
w := approvedGroup(t)
allGreen(w)
w.forge.refuseRepo = map[string]error{gCtl: refusedWords("novox/mesh-controller#1")}
w.op.answer(w.op.open()[0], OptionApprove)
w.settleAll()
w.h.PullMerged(merged(gCat, 2, gHk, gT1))
for range 3 {
w.later(time30s)
w.settleAll()
lines := urgentLines(w)
if len(lines) != 1 || lines[0].ID != IDOf(gCtl, gHc) || !strings.Contains(lines[0].Says, "feat/g") {
t.Fatalf("the half merged group is not raised: %+v", lines)
}
}
// A half merged group with no refusal on record (the pass died between members) is raised too.
d := w.delivery(IDOf(gCtl, gHc))
d.Ready.RefusedSince = nil
if lines := urgentLines(w); len(lines) != 1 || !strings.Contains(lines[0].Says, "half merged") {
t.Fatalf("a half merged group without a refusal is not raised: %+v", lines)
}
w.forge.refuseRepo = nil
setChecks(w, gCtl, gHc, "success", "success")
w.h.PullMerged(merged(gCtl, 1, gHc, gT1))
w.settleAll()
if lines := urgentLines(w); len(lines) != 0 {
t.Fatalf("the group is whole and still raised: %+v", lines)
}
}
// 1275/AC1: a group with a member whose required status is not a success is not asked its Approve; the group says which
// member and status; it is asked once every member may merge.
func TestAGroupIsNotAskedWhileAMembersRequiredStatusIsNotASuccess(t *testing.T) {
w := newWorld(t)
w.op.auto = false
w.h.PullUpdated(pr(gCtl, 1, gHc, "feat/g", "internal/a.go"))
w.h.PullUpdated(pr(gCat, 2, gHk, "feat/g", "modules/app/module.json"))
w.h.Checked(judged(gCtl, 1, gHc, "pass", gTrunk0))
w.h.Checked(judged(gCat, 2, gHk, "pass", gTrunk0))
w.settleAll()
g := w.h.groups["feat/g"]
setChecks(w, gCat, gHk, "success", "success") // the composed verdict set every member's gate
setChecks(w, gCtl, gHc, "success", "pending")
w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed",
Members: []struct {
Owner string `json:"owner"`
Repo string `json:"repo"`
Commit string `json:"commit"`
Trunk string `json:"trunk,omitempty"`
}{{Owner: "novox", Repo: "mesh-controller", Commit: gHc, Trunk: gTrunk0[:12]},
{Owner: "novox", Repo: "mesh-catalog", Commit: gHk, Trunk: gTrunk0[:12]}}})
w.settleAll()
if w.op.count() != 0 {
t.Fatalf("the operator was asked with a required status pending: %d asks", w.op.count())
}
if g.Hold == nil || g.Hold.Kind != "unasked" || g.Hold.Member != IDOf(gCtl, gHc) || !strings.Contains(g.Hold.Says, "mesh/repo-check (pending)") {
t.Fatalf("the group does not say which member and status it waits for: %+v", g.Hold)
}
setChecks(w, gCtl, gHc, "success", "success")
w.later(time30s)
w.settleAll()
if w.op.count() != 1 || g.Hold != nil {
t.Fatalf("asks %d, hold %+v; wanted the group asked once its statuses are green", w.op.count(), g.Hold)
}
}
// 1275/AC5: a member's required status turning not-success while the ask waits withdraws the ask at the router; the
// answer to the withdrawn ask merges nothing; the group is asked again once every member may merge.
func TestAStatusThatTurnsWhileTheAskWaitsWithdrawsItAndItsAnswerMergesNothing(t *testing.T) {
w := approvedGroup(t)
allGreen(w)
ask := w.op.open()[0]
w.h.CommitStatus(ForgeStatusEvent{Owner: "novox", Repo: "mesh-catalog", SHA: gHk, Context: repoCheck, State: "pending"})
setChecks(w, gCat, gHk, "success", "pending")
w.settleAll()
if len(w.op.cancelled) != 1 || w.op.cancelled[0] != ask {
t.Fatalf("the ask was not withdrawn at the router: %v", w.op.cancelled)
}
w.op.answer(ask, OptionApprove) // an answer that arrives late
w.settleAll()
if len(w.forge.merges) != 0 {
t.Fatalf("the answer to a withdrawn ask merged: %v", w.forge.merges)
}
if w.op.count() != 1 {
t.Fatalf("asked again while a status was still pending: %d", w.op.count())
}
setChecks(w, gCat, gHk, "success", "success")
w.later(time30s)
w.settleAll()
if w.op.count() != 2 {
t.Fatalf("not asked again once every member may merge: %d", w.op.count())
}
}
// 1276/AC1: given a closed group whose member's merge was refused with "not a success: mesh/repo-check (missing)", then
// the member's own check is asked, not the composed check.
func TestAClosedGroupsMemberRefusedForItsOwnMissingStatusAsksItsOwnCheck(t *testing.T) {
w := approvedGroup(t)
allGreen(w)
w.forge.refuseRepo = map[string]error{gCtl: refusedWords("novox/mesh-controller#1")}
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)
}
}
// 1275/AC3 (review round 1): a member that is both refused and held for a required status is said by one stalled line,
// not two with the same id.
func TestAMemberBothRefusedAndHeldForAStatusIsOneStalledLine(t *testing.T) {
w := approvedGroup(t)
allGreen(w)
w.forge.refuseRepo = map[string]error{gCat: refusedWords("novox/mesh-catalog#2")}
w.op.answer(w.op.open()[0], OptionApprove)
w.settleAll()
setChecks(w, gCat, gHk, "success", "") // now its status is missing too; the next pass is held
w.h.CommitStatus(ForgeStatusEvent{Owner: "novox", Repo: "mesh-catalog", SHA: gHk, Context: repoCheck, State: "success"})
w.h.mergeAgainNow(w.delivery(IDOf(gCat, gHk)))
w.later(time30s)
w.settleAll()
if g := w.h.groups["feat/g"]; g.Hold == nil {
t.Fatalf("the group was not held for the missing status")
}
n := 0
for _, l := range w.h.Stalled() {
if l.ID == IDOf(gCat, gHk) {
n++
}
}
if n != 1 {
t.Fatalf("%d stalled lines for one member: %+v", n, w.h.Stalled())
}
}
@@ -968,12 +968,24 @@ type Group struct {
Closed bool `json:"closed,omitempty"`
// Said is the derived state last said, so a change of it is said once.
Said string `json:"said,omitempty"`
// Hold is why the group's merge or its Approve's ask is held back by a member's required status that is not a
// success (issue 1275); nil when nothing is.
Hold *GroupHold `json:"hold,omitempty"`
// SaidStatus is the mesh/delivery-group status last written for it: a change of the status a state takes is
// written too, though the state did not change (novox/hq issue 902).
SaidStatus string `json:"said_status,omitempty"`
Owed []Effect `json:"owed,omitempty"`
}
// GroupHold is a group held back from its Approve or its merge because a member's required status is not a success.
type GroupHold struct {
// Kind is "unasked" (the Approve is not asked) or "blocked" (the approved merge is not started: nothing is merged).
Kind string `json:"kind"`
Member string `json:"member"`
Says string `json:"says"`
Since time.Time `json:"since"`
}
// GroupCheck is a group's composed check.
type GroupCheck struct {
Asked string `json:"asked"`
@@ -202,6 +202,13 @@ func (h *Holder) readyAsks(now time.Time) []func() {
if !r.LastTry.IsZero() {
continue // refused: asked again at the next event that decides whether it may merge (mergeAgainNow), with no timer
}
if g := h.groups[d.Group]; d.Group != "" && g != nil {
// A group is merged in one pass, in its order, and stops at the first refusal (issue 1275).
if a := h.groupPass(g, d, now); a != nil {
asks = append(asks, a)
}
continue
}
r.Tries++
r.LastTry, r.merging = now, true
h.keep(d)
@@ -330,48 +337,52 @@ func (h *Holder) groupTurnToMerge(d *Delivery) string {
// the heads the Approve covers, each `owner/repo@<full sha>` (novox/hq ADR 0421 §9), and refuses a pull request whose
// head differs from the one approved.
func (h *Holder) askMerge(id string, heads map[string]string) func() {
return func() {
h.mu.Lock()
d := h.deliveries[id]
if d == nil || d.Ready == nil {
h.mu.Unlock()
return
}
owner, repo, number, head := d.Owner(), d.Repo(), d.Number, d.Commit
if sha := heads[id]; sha != "" {
head = sha
}
return func() { h.mergeOne(id, heads) }
}
// mergeOne asks the merge tool for one approved head and keeps its answer; whether it merged.
func (h *Holder) mergeOne(id string, heads map[string]string) bool {
h.mu.Lock()
d := h.deliveries[id]
if d == nil || d.Ready == nil {
h.mu.Unlock()
err := h.Forge.Merge(owner, repo, number, head, approvedHeads(heads))
h.mu.Lock()
defer h.mu.Unlock()
d = h.deliveries[id]
if d == nil || d.Ready == nil {
return
}
r, now := d.Ready, h.Now()
r.merging = false
if err == nil {
// Issue 1025: the announcement is owed, kept with the delivery, so a death right after the merge
// still says it once.
d.Owed = append(d.Owed, Effect{Kind: EffectEmit, Event: "merged", Line: string(mustJSON(mergedEvent(d, head))), Since: now})
r.Merged, r.Answer, r.RefusedSince = &now, "merged", nil
h.Logf("[mesh-delivery] %s merged on the operator's Approve at %s, its check passed against its trunk's head", id,
shortOf(head, 12))
h.keep(d)
return
}
answer := oneLine(err.Error())
if r.RefusedSince == nil {
r.RefusedSince = &now
}
if answer != r.Answer {
h.Logf("[mesh-delivery] %s is approved, and the merge tool did not merge it (asked again at the next verdict, Approve or composed check): %s", id,
answer)
}
r.Answer = answer
h.keep(d)
return false
}
owner, repo, number, head := d.Owner(), d.Repo(), d.Number, d.Commit
if sha := heads[id]; sha != "" {
head = sha
}
h.mu.Unlock()
err := h.Forge.Merge(owner, repo, number, head, approvedHeads(heads))
h.mu.Lock()
defer h.mu.Unlock()
d = h.deliveries[id]
if d == nil || d.Ready == nil {
return false
}
r, now := d.Ready, h.Now()
r.merging = false
if err == nil {
// Issue 1025: the announcement is owed, kept with the delivery, so a death right after the merge
// still says it once.
d.Owed = append(d.Owed, Effect{Kind: EffectEmit, Event: "merged", Line: string(mustJSON(mergedEvent(d, head))), Since: now})
r.Merged, r.Answer, r.RefusedSince = &now, "merged", nil
h.Logf("[mesh-delivery] %s merged on the operator's Approve at %s, its check passed against its trunk's head", id,
shortOf(head, 12))
h.keep(d)
return true
}
answer := oneLine(err.Error())
if r.RefusedSince == nil {
r.RefusedSince = &now
}
if answer != r.Answer {
h.Logf("[mesh-delivery] %s is approved, and the merge tool did not merge it (asked again at the next verdict, Approve or composed check): %s", id,
answer)
}
r.Answer = answer
h.keep(d)
return false
}
// approvedHeads are the heads an Approve covers as the merge tool is handed them: `owner/repo@<full sha>`, sorted.
@@ -420,17 +431,19 @@ func (h *Holder) readyLines(now time.Time) []StalledLine {
// 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)})
H2: "none: the group's composed check is asked again after each refusal, and the merge each minute — the operator's",
Severity: "urgent",
Says: fmt.Sprintf("group %s is half merged: %s#%d is approved at %s and the merge tool has refused it since %s: %s. "+
"Needed: the refusal's cause is met (the merge is asked again at each verdict, status or Approve), or the operator decides.",
d.Group, d.Repository, d.Number, shortOf(r.Head, 12), 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(),
H2: "none: the merge is asked again on a verdict, an Approve, a group's composed check or a merge check's " +
"status turning success — the operator's",
Says: fmt.Sprintf("%s#%d is approved at %s, and the merge tool has refused it since %s: %s", d.Repository,
d.Number, shortOf(r.Head, 12), r.RefusedSince.UTC().Format(time.RFC3339), r.Answer)})
Says: fmt.Sprintf("%s%s#%d is approved at %s, and the merge tool has refused it since %s: %s%s", heldInGroup(d),
d.Repository, d.Number, shortOf(r.Head, 12), r.RefusedSince.UTC().Format(time.RFC3339), r.Answer, neededOf(d)),
Severity: severityOf(d)})
}
}
return out
@@ -452,3 +465,27 @@ func mergedEvent(d *Delivery, head string) map[string]any {
}
return ev
}
// heldInGroup is the words that put a refused member in its group's pass: none for a lone delivery.
func heldInGroup(d *Delivery) string {
if d.Group == "" {
return ""
}
return "group " + d.Group + " is held, and no member after this one is merged: "
}
// neededOf is what a refused member needs, for a member of a group (issue 1275).
func neededOf(d *Delivery) string {
if d.Group == "" {
return ""
}
return ". Needed: the refusal's cause is met (the merge is asked again at each verdict, status or Approve), or the operator decides."
}
// severityOf is "urgent" for a member of a group (a refused merge is the group held), empty for a lone delivery.
func severityOf(d *Delivery) string {
if d.Group == "" {
return ""
}
return "urgent"
}
@@ -226,6 +226,8 @@ type StalledLine struct {
Bound string `json:"bound"`
H2 string `json:"h2"`
Says string `json:"says"`
// Severity is "urgent" for a group that is held or half merged (issue 1275); empty otherwise.
Severity string `json:"severity,omitempty"`
}
// Stalled is the `stalled` verb: what the controller's self-check raises as `delivery.<id>.stalled`.
@@ -269,6 +271,7 @@ func (h *Holder) Stalled() []StalledLine {
out = append(out, h.withChecks(h.uncheckedLines(now))...)
out = append(out, h.orphanLines(now)...)
out = append(out, h.readyLines(now)...)
out = append(out, h.groupLines(now)...)
sort.Slice(out, func(i, j int) bool { return out[i].ID < out[j].ID })
return out
}