Merge pull request 'The merge Approve says the check's real state, and is asked again when it passes or fails (issue 1733)' (#514) from fix/1733-ask-says-the-check into main

modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go

@@ -148,8 +148,32 @@ func touchesSaid(d *Delivery) string {
 	return strings.Join(d.Check.Modules, ", ")
 }
 
+// checkClause is the check's state as the ask's summary says it after the pull request's or the group's name: sep and
+// "passed its check", or ": its check has no verdict yet: it is running (since 21:05 UTC)" and the like.
+func checkClause(c checkSaid, sep string) string {
+	if c.passed() {
+		return sep + c.Words
+	}
+	return ": " + c.Words
+}
+
+// approveCaveat is what an Approve does beyond the merge while the check has not passed: it merges before the check ends,
+// or although it failed (the check gates no merge, check-gates-merge false). Empty once it passed.
+func approveCaveat(c checkSaid) string {
+	switch c.Key {
+	case checkSaidPass:
+		return ""
+	case checkSaidFail:
+		return " although its check failed"
+	}
+	return " now, before its check ends"
+}
+
 // composeMergeAsk is the ask for a delivery, or for a group's members together.
-func composeMergeAsk(id string, d *Delivery, group string, members []*Delivery, heads map[string]string, now time.Time) asks.Ask {
+// check is the state of the check at the heads it covers (checkAt): the ask says "passed its check" only when it did, and
+// otherwise that Approve merges before the check ends, or although it failed (issue 1733).
+func composeMergeAsk(id string, d *Delivery, group string, members []*Delivery, heads map[string]string, check checkSaid,
+	now time.Time) asks.Ask {
 	var a asks.Ask
 	a.ID, a.Who, a.Expires = id, asks.Operator, now.Add(AskLasts)
 	a.OnExpiry = "nothing is merged; the ask is made again"
@@ -170,8 +194,8 @@ func composeMergeAsk(id string, d *Delivery, group string, members []*Delivery,
 	}
 	if group == "" {
 		a.Headline = phone("Merge into main: "+pullSaid(d)+" "+d.Title, asks.HeadlineLength)
-		summary := fmt.Sprintf("%s %s passed its checks. Approve merges it into main at %s; Decline leaves it unmerged.",
+		summary := fmt.Sprintf("%s %s%s. Approve merges it into main at %s%s; Decline leaves it unmerged.",
-			pullSaid(d), d.Title, shortOf(d.Commit, 12))
+			pullSaid(d), d.Title, checkClause(check, " "), shortOf(d.Commit, 12), approveCaveat(check))
 		if unreviewed != "" {
 			summary = strings.ToUpper(unreviewed[:1]) + unreviewed[1:] + ". " + summary
 		}
@@ -187,8 +211,12 @@ func composeMergeAsk(id string, d *Delivery, group string, members []*Delivery,
 			names = append(names, pullSaid(m))
 		}
 		a.Headline = phone("Merge into main: group "+group+" ("+strings.Join(names, ", ")+")", asks.HeadlineLength)
-		summary := fmt.Sprintf("The group %s, %d pull requests, passed its checks. Approve merges all of them into main in one pass, at the heads named; Decline leaves them unmerged.",
+		caveat := approveCaveat(check)
-			group, len(members))
+		if caveat != "" {
+			caveat = "," + caveat
+		}
+		summary := fmt.Sprintf("The group %s, %d pull requests%s. Approve merges all of them into main in one pass, at the heads named%s; Decline leaves them unmerged.",
+			group, len(members), checkClause(check, ", "), caveat)
 		if unreviewed != "" {
 			summary = strings.ToUpper(unreviewed[:1]) + unreviewed[1:] + ". " + summary
 		}
@@ -309,6 +337,10 @@ func (h *Holder) endStaleAsks() {
 			h.cancels = append(h.cancels, h.dropAsk(r.AskID, "its head "+shortOf(d.Commit, 12)+" is no longer the pull request's head, or the pull request closed"))
 		case !h.headsHold(d, r):
 			h.cancels = append(h.cancels, h.dropAsk(r.AskID, "a head it covers moved on: the heads it covers are no longer the group's heads"))
+		case r.Approved == nil && r.AskID != "" && r.AskCheck != "" && !h.checkGates() && h.checkMoved(d, r.AskCheck):
+			// The check gates no merge, so the ask was made before its verdict: once the check passes or fails (or a pass it
+			// named is gone), the ask is asked again in words that say so (issue 1733). Running and not started say the same.
+			h.cancels = append(h.cancels, h.withdrawAsk(r.AskID, "the check at its heads is now: "+h.checkAt(h.askMembers(d)).Words))
 		case r.Approved == nil && r.AskID != "" && !r.AskedAt.IsZero() && !h.Now().Before(r.AskedAt.Add(AskLasts)):
 			// The ask lapsed at the router with nobody's answer, and nothing in the gate's inputs says so: it is taken away and
 			// written again, which the teller asks afresh (ADR 0441 §1).
@@ -317,6 +349,16 @@ func (h *Holder) endStaleAsks() {
 	}
 }
 
+// checkMoved is whether the check at the heads of d's ask reached a verdict its pending ask does not say: it passed or failed
+// since, or the pass it said is gone. Under the lock.
+func (h *Holder) checkMoved(d *Delivery, said string) bool {
+	now := h.checkAt(h.askMembers(d)).Key
+	if now == said {
+		return false
+	}
+	return now == checkSaidPass || now == checkSaidFail || said == checkSaidPass || said == checkSaidFail
+}
+
 // approval is what a delivery that may merge waits on for the operator: empty once its head is approved, and the ask
 // to make when none is made yet. Under the lock.
 func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) {
@@ -367,19 +409,20 @@ func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) {
 			asking = max(asking, m.Ready.Asking+1)
 		}
 	}
-	ask := composeMergeAsk(sdk.AskingID(id, asking), d, group, members, heads, now)
+	check := h.checkAt(members)
+	ask := composeMergeAsk(sdk.AskingID(id, asking), d, group, members, heads, check, 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)
-		m.Ready.AskedKey, m.Ready.Asking = id, asking
+		m.Ready.AskedKey, m.Ready.Asking, m.Ready.AskCheck = id, asking, check.Key
 		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)",
+	h.Logf("[mesh-delivery] %s (%s): the operator is asked to Approve its merge into main (%s, ask %d)",
-		d.ID, id, r.Asks)
+		d.ID, check.Words, id, r.Asks)
 	gid := ""
 	if group != "" {
 		gid = d.Group
@@ -432,7 +475,7 @@ func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) {
 			h.mu.Lock()
 			defer h.mu.Unlock()
 			undo()
-			h.Logf("[mesh-delivery] %s passed its checks and the operator is not asked: %s", d.ID, says)
+			h.Logf("[mesh-delivery] %s (%s): the operator is not asked: %s", d.ID, check.Words, says)
 			h.lookAt(h.Now(), "an ask not made: the forge account cannot write a repository it covers")
 			return
 		}

modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go

@@ -287,7 +287,7 @@ func (h *Holder) ForgeMerged(f sdk.Forge) {
 	r, now := d.Ready, h.Now()
 	d.Owed = append(d.Owed, Effect{Kind: EffectEmit, Event: "merged", Line: string(mustJSON(mergedEvent(d, f.Head))), Since: now})
 	r.Merged, r.Answer, r.RefusedSince, r.merging, r.MergeCommit = &now, "merged", nil, false, f.Merge
-	h.Logf("[mesh-delivery] %s merged on the operator's Approve at %s, its check passed against its trunk's head", d.ID, shortOf(f.Head, 12))
+	h.Logf("[mesh-delivery] %s merged on the operator's Approve at %s (%s)", d.ID, shortOf(f.Head, 12), h.checkAt(h.askMembers(d)).Words)
 	h.keep(d)
 }
 

modules/mesh-delivery-gate/cmd/mesh-delivery-gate/onestatus.go

@@ -302,3 +302,72 @@ func checkerRefusal(id string) error {
 		"its trunk or its group, and nothing is asked of it here (novox/hq ADR 0447). Read where its check stands with the "+
 		"checker's checker_pull tool", id)
 }
+
+// The states of the check at an ask's heads, as the ask says them (issue 1733).
+const (
+	checkSaidPass    = "pass"
+	checkSaidRunning = "running"
+	checkSaidNone    = "none"
+	checkSaidFail    = "fail"
+)
+
+// checkSaid is the state of the check at the heads an ask covers, and its words for the ask: what the checker's verdict at
+// exactly those heads says, never what the gate would like it to say (issue 1733). Key is one of the checkSaid states.
+type checkSaid struct {
+	Key   string
+	Words string
+}
+
+// passed is whether the check at the heads passed.
+func (c checkSaid) passed() bool { return c.Key == checkSaidPass }
+
+// checkAt reads the check at these members' heads: the checker's verdict for exactly those heads (its verdicts bucket,
+// keyed by their hash), and, while none is held, the forge's mesh/check at a member's head (a failure or a pending one
+// there). "passed its check" is said only for a passing verdict at these heads. Under the lock.
+func (h *Holder) checkAt(members []*Delivery) checkSaid {
+	v, hash, ok := h.checkerAt(members)
+	if ok {
+		switch v.State {
+		case checkerPass:
+			return checkSaid{Key: checkSaidPass, Words: "passed its check"}
+		case checkerRunning:
+			words := "its check has no verdict yet: it is running"
+			if since, held := h.runningSince[hash]; held && !since.IsZero() {
+				words += " (since " + since.UTC().Format("15:04") + " UTC)"
+			}
+			return checkSaid{Key: checkSaidRunning, Words: words}
+		default:
+			why := firstOf(oneLine(v.Failed.Test), oneLine(v.Failed.Words), v.State)
+			if v.Failed.Test != "" && v.Failed.Words != "" {
+				why = oneLine(v.Failed.Test) + ": " + oneLine(v.Failed.Words)
+			}
+			return checkSaid{Key: checkSaidFail, Words: "its check failed: " + clip(why, 120)}
+		}
+	}
+	for _, m := range members {
+		switch m.CheckState {
+		case "failure", "error":
+			why := "mesh/check is " + m.CheckState + " at " + shortOf(m.Commit, 12)
+			if m.Check != nil && m.Check.Summary != "" {
+				why = oneLine(m.Check.Summary)
+			}
+			return checkSaid{Key: checkSaidFail, Words: "its check failed: " + clip(why, 120)}
+		}
+	}
+	succeeded := 0
+	for _, m := range members {
+		switch m.CheckState {
+		case "pending":
+			return checkSaid{Key: checkSaidRunning, Words: "its check has no verdict yet: it is running"}
+		case "success":
+			succeeded++
+		}
+	}
+	switch {
+	case succeeded > 0 && len(members) == 1:
+		return checkSaid{Key: checkSaidPass, Words: "passed its check"} // its head's mesh/check is the checker's verdict at it
+	case succeeded == len(members) && succeeded > 0:
+		return checkSaid{Key: checkSaidNone, Words: "its members' own checks passed; the check of their heads together has not answered"}
+	}
+	return checkSaid{Key: checkSaidNone, Words: "its check has no verdict yet"}
+}

modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go

@@ -115,6 +115,10 @@ type ReadyRun struct {
 	Heads   map[string]string `json:"heads,omitempty"`
 	AskedAt time.Time         `json:"asked_at"`
 	Asks    int               `json:"asks,omitempty"`
+	// AskCheck is the state of the check at the heads the pending ask names, as its words said it (checkAt: pass, running,
+	// none or fail; issue 1733): while the check gates no merge, an ask whose check reached a verdict since is asked again
+	// in new words, so it never keeps a claim the check no longer holds.
+	AskCheck string `json:"ask_check,omitempty"`
 	// Approved and Declined are when the operator chose, Warrant what the router's warrant says of who and how; Refused
 	// why an answer was not taken (it did not hold to the ask).
 	Approved *time.Time `json:"approved,omitempty"`
@@ -186,7 +190,8 @@ func (h *Holder) readyAsks(now time.Time) []func() {
 		if d.Ready == nil && d.State == Ready {
 			h.runOf(d, now)
 			h.Poke() // its check, if none is asked, is worth asking now (askCheck)
-			h.Logf("[mesh-delivery] %s: its checks passed; the operator's Approve of its merge into main is asked once it may merge", d.ID)
+			h.Logf("[mesh-delivery] %s is ready (%s); the operator's Approve of its merge into main is asked once it may merge",
+				d.ID, h.checkAt([]*Delivery{d}).Words)
 		}
 		if a := h.sayAfterLine(d); a != nil {
 			asks = append(asks, a)
@@ -227,11 +232,12 @@ func (h *Holder) readyAsks(now time.Time) []func() {
 		if d.Base != MergeBranch {
 			if r.OtherBase != d.Base {
 				r.OtherBase = d.Base
-				h.Logf("[mesh-delivery] %s passed its checks and is not asked: it merges into %s, not %s", d.ID, d.Base, MergeBranch)
+				check := h.checkAt([]*Delivery{d}).Words // ready need not mean passed: the check may gate no merge (issue 1733)
+				h.Logf("[mesh-delivery] %s (%s) is not asked: it merges into %s, not %s", d.ID, check, d.Base, MergeBranch)
 				h.keep(d)
-				asks = append(asks, h.sayOn(d, fmt.Sprintf("mesh-delivery: **not merged.** This pull request's checks passed at "+
+				asks = append(asks, h.sayOn(d, fmt.Sprintf("mesh-delivery: **not merged.** This pull request is at %s (%s), "+
-					"%s, and it merges into %s. Only a merge into %s is this owner's; a merge into another branch is its "+
+					"and it merges into %s. Only a merge into %s is this owner's; a merge into another branch is its "+
-					"author's (novox/hq ADR 0398 §4).", d.Commit, d.Base, MergeBranch)))
+					"author's (novox/hq ADR 0398 §4).", d.Commit, check, d.Base, MergeBranch)))
 			}
 			continue
 		}
@@ -524,13 +530,14 @@ func (h *Holder) readyLines(now time.Time) []StalledLine {
 		case d.Base == "" && d.Number != 0 && now.Sub(r.Seen) >= headHeardBound:
 			out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State, For: now.Sub(r.Seen).Round(time.Second).String(),
 				Bound: headHeardBound.String(), H2: "none: a later announcement of the pull request's head (the merge tool announces it again, the forge's announcer restarts, or a new push) or an edit of its description; a recheck does not fill it — the operator's",
-				Says: fmt.Sprintf("%s#%d passed its checks at %s and waits for %s", d.Repository, d.Number, shortOf(r.Head, 12), waitsForHead)})
+				Says: fmt.Sprintf("%s#%d at %s (%s) waits for %s", d.Repository, d.Number, shortOf(r.Head, 12),
+					h.checkAt([]*Delivery{d}).Words, waitsForHead)})
 		case r.CannotWrite != "" && r.Approved == nil && r.CannotWriteSince != nil:
 			out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State,
 				For: now.Sub(*r.CannotWriteSince).Round(time.Second).String(), Bound: "0s",
 				H2: "none: the forge account given write on the repository (gitea_grant_write; the gitea module grants it itself once a module of it is registered), which asks the operator then",
-				Says: fmt.Sprintf("%s#%d passed its checks at %s and the operator is not asked for its Approve: %s",
+				Says: fmt.Sprintf("%s#%d at %s (%s): the operator is not asked for its Approve: %s",
-					d.Repository, d.Number, shortOf(r.Head, 12), r.CannotWrite)})
+					d.Repository, d.Number, shortOf(r.Head, 12), h.checkAt([]*Delivery{d}).Words, r.CannotWrite)})
 		case r.Failed != "" && d.State == Rejected:
 			// Its cure is its author's new head: listed here at once (`show`), told on the pull request at once, and the
 			// operator's only once no new head came within the author's bound (novox/hq ADR 0505 §5).

modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready_test.go

@@ -1,11 +1,15 @@
 package main
 
 import (
+	"encoding/json"
 	"errors"
 	"sort"
 	"strings"
 	"testing"
 	"time"
+
+	"git.novox.be/novox/mesh-sdk/go/asks"
+	sdk "git.novox.be/novox/mesh-sdk/go/delivery"
 )
 
 // **A pull request whose checks passed and whose merge the operator approved is merged by this owner** (novox/hq issues
@@ -65,6 +69,73 @@ func TestAnApprovedPullRequestIsMergedThroughTheMergeTool(t *testing.T) {
 		t.Fatalf("merged again: %v", w.forge.merges)
 	}
 
+	// 1733: the Approve's ask says the check's state at the heads it names, read from the checker's verdict there: "passed
+	// its check" only when it passed. While the check gates no merge, the ask is made before any verdict, says that Approve
+	// merges before the check ends, and is asked again in new words once the check passes or fails; a running check is no
+	// new word.
+	for _, row := range []struct {
+		name, state, failed string
+		gated               bool
+		asks                int
+		want, not           []string
+	}{
+		{name: "gated, passed", gated: true, state: "pass", asks: 1,
+			want: []string{"app#7 Fix it passed its check. Approve merges it into main at " + readyHead[:12] + ";"},
+			not:  []string{"before its check ends", "no verdict yet"}},
+		{name: "ungated, no verdict", asks: 1,
+			want: []string{"its check has no verdict yet", "Approve merges it into main at " + readyHead[:12] + " now, before its check ends"},
+			not:  []string{"passed"}},
+		{name: "ungated, running", state: "running", asks: 1,
+			want: []string{"its check has no verdict yet", "before its check ends"}, not: []string{"passed"}},
+		{name: "ungated, then passed", state: "pass", asks: 2,
+			want: []string{"passed its check. Approve merges it into main at " + readyHead[:12] + ";"}, not: []string{"before its check ends"}},
+		{name: "ungated, then failed", state: "fail", failed: "TestThing: wanted 1, got 2", asks: 2,
+			want: []string{"its check failed: TestThing: wanted 1, got 2", "although its check failed"}, not: []string{"passed"}},
+	} {
+		t.Run(row.name, func(t *testing.T) {
+			v := newWorld(t)
+			if !row.gated {
+				ungated(v)
+			}
+			v.op.auto = false
+			p := pr("novox/app", 7, readyHead, "feat/x", "src/a.go")
+			p.Title = "Fix it"
+			v.h.PullUpdated(p)
+			put := func() {
+				hash, _ := sdk.HeadsHash([]sdk.Head{headOf("novox/app", 7, readyHead)})
+				value, _ := json.Marshal(map[string]any{"v": 1, "group": "g", "heads": hash, "state": row.state,
+					"failed": map[string]string{"words": row.failed}})
+				v.h.CheckerVerdictChanged("g."+hash, value)
+			}
+			if row.gated {
+				v.h.Checked(judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1"))
+			}
+			v.settleAll()
+			if !row.gated && row.state != "" {
+				put()
+				v.settleAll()
+			}
+			open := v.op.open()
+			if len(open) != 1 || v.op.count() != row.asks {
+				t.Fatalf("open asks %v, asked %d times; wanted one open, asked %d times", open, v.op.count(), row.asks)
+			}
+			e := v.op.ask(open[0]).Explanation
+			for _, w := range row.want {
+				if !strings.Contains(e, w) {
+					t.Fatalf("the ask does not say %q:\n%s", w, e)
+				}
+			}
+			for _, w := range row.not {
+				if strings.Contains(e, w) {
+					t.Fatalf("the ask says %q:\n%s", w, e)
+				}
+			}
+			if err := asks.Glance(e); err != nil {
+				t.Fatal(err)
+			}
+		})
+	}
+
 	// check-gates-merge false (the operator, 2026-10-25 22:56): a reviewed pull request is asked with no verdict, another
 	// merge moving its trunk puts it out of nothing, a failed verdict is shown on it and holds nothing, and the gate's
 	// status and show say the mode.
@@ -255,6 +326,11 @@ func TestAGroupMergesEveryMemberInOnePassOnlyOnceComposedAndApproved(t *testing.
 	if u.op.count() != 1 {
 		t.Fatalf("ungated, the group is asked %d times; wanted once", u.op.count())
 	}
+	// 1733: the group's ask claims no pass it has not had.
+	if e := u.op.ask(u.op.open()[0]).Explanation; strings.Contains(e, "passed") ||
+		!strings.Contains(e, "2 pull requests: its check has no verdict yet. Approve merges all of them into main in one pass, at the heads named, now, before its check ends;") {
+		t.Fatalf("ungated, the group's ask does not say its check has no verdict yet:\n%s", e)
+	}
 	u.op.answer(u.op.open()[0], OptionApprove)
 	u.settleAll()
 	if len(u.forge.merges) != 2 {