mesh-delivery-gate: the Approve's ask says the check's actual state at its heads, never a pass it has not had (issue 1733)
mesh/delivery merged: source.main moved to novox/mesh-catalog@53bf4bb84cb1
mesh/check checking 099605ec70a2 on 53bf4bb84cb1

With check-gates-merge false a head is ready and asked before any verdict exists, yet the ask said "<pr> passed its
checks" (a group: "passed its checks"): on 2026-10-26 the operator approved mesh-delivery #79 and a six-repository group
on that claim while their checks had not answered.

The ask now reads the checker's verdict at exactly the heads it covers (checkAt, beside checkerAt): "passed its check"
only for a pass there; otherwise "its check has no verdict yet[: it is running (since HH:MM UTC)]" or "its check failed:
<first failure>", and the Approve clause says it merges "now, before its check ends" or "although its check failed".
While the check gates no merge, a pending ask whose check then passes or fails (or loses a pass it named) is withdrawn
and asked again in new words (the ask's AskCheck against the state now, endStaleAsks, the review-record path's
withdrawAsk); a check that starts running is no new word, so the wait stays one message. Whether a merge waits is
unchanged. The gate's log lines and stalled lines that claimed a pass for a ready head say the same state.

Quick fix: replace "passed its checks" by neutral words in the two summaries. Thorough fix: one reader of the check's
state at the ask's heads (checkAt) used by every text that names it, and the ask re-said when that state changes.
Chosen: thorough, as it costs one helper and one case in endStaleAsks, and the quick one would still leave a pending
ask silent about a failure that came after it.

Tests: rows in TestAnApprovedPullRequestIsMergedThroughTheMergeTool (gated pass; ungated no verdict, running, then
passed, then failed) and the ungated group's wording; no new test function.
This commit is contained in:
2026-10-26 22:18:08 +01:00
parent 78d7ef71cc
commit 099605ec70
5 changed files with 214 additions and 19 deletions
@@ -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.",
pullSaid(d), d.Title, shortOf(d.Commit, 12))
summary := fmt.Sprintf("%s %s%s. Approve merges it into main at %s%s; Decline leaves it unmerged.",
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.",
group, len(members))
caveat := approveCaveat(check)
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)",
d.ID, id, r.Asks)
h.Logf("[mesh-delivery] %s (%s): the operator is asked to Approve its merge into main (%s, ask %d)",
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
}
@@ -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)
}
@@ -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"}
}
@@ -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 "+
"%s, 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)))
asks = append(asks, h.sayOn(d, fmt.Sprintf("mesh-delivery: **not merged.** This pull request is at %s (%s), "+
"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, 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",
d.Repository, d.Number, shortOf(r.Head, 12), r.CannotWrite)})
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), 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).
@@ -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 {