mesh-delivery-gate: while the check gates no merge, ask the Approve only once the check has a verdict (issue 1733)

The operator, 2026-10-27 23:54: "if you don't want me to approve it, don't send
the approval request". The gate asked the Approve the moment a pull request
opened, before its check answered, and five changes were approved unchecked.

Now the ask waits for the checker's verdict at the heads it covers (a group's
verdict of all its heads together; a placement change's verdict ends only once
its nodes answered, ADR 0559 §5). A pass or a failure is asked, the failure in
its words, since check-gates-merge false lets the operator merge it anyway. A
check with no verdict 60 minutes after the heads became ready is asked anyway,
saying so, and asked again in new words when its verdict comes.

Quick fix (only the ask's wording) against thorough (the one place that decides
when to ask, ungatedWaits): thorough, which is one place here.

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

@@ -411,6 +411,9 @@ func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) {
 		}
 	}
 	check := h.checkAt(members)
+	if !h.checkGates() {
+		check = unansweredWords(check) // asked with no verdict only past its bound (verdictWaits)
+	}
 	ask := composeMergeAsk(sdk.AskingID(id, asking), d, group, members, heads, check, now)
 	taken := []*Delivery{}
 	var reads []memberRead

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

@@ -1,6 +1,11 @@
 package main
 
-import "time"
+import (
+	"encoding/json"
+	"time"
+
+	sdk "git.novox.be/novox/mesh-sdk/go/delivery"
+)
 
 var t0 = time.Date(2026, 10, 6, 12, 0, 0, 0, time.UTC)
 
@@ -27,3 +32,12 @@ func statusOn(w *world, repo, commit string) string {
 	defer w.forge.mu.Unlock()
 	return w.forge.statuses[repo+"@"+commit+" mesh/delivery"]
 }
+
+// checkerSays puts the checker's verdict at these heads together into its verdicts bucket, as captured on the bus
+// (mesh-delivery-checker, key `<group>.<heads hash>`).
+func checkerSays(w *world, group, state, failed string, heads ...sdk.Head) {
+	hash, _ := sdk.HeadsHash(heads)
+	value, _ := json.Marshal(map[string]any{"v": 1, "group": group, "heads": hash, "state": state,
+		"failed": map[string]string{"words": failed}})
+	w.h.CheckerVerdictChanged(group+"."+hash, value)
+}

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

@@ -446,9 +446,62 @@ func (h *Holder) ungatedWaits(d *Delivery) string {
 			}
 		}
 	}
+	if h.covering(d) == nil {
+		return h.verdictWaits(d)
+	}
 	return ""
 }
 
+// askCheckBound is how long an Approve waits for the check's verdict at its heads while the check gates no merge: past it,
+// the operator is asked anyway, and the ask says the check has not answered. The checker's own step bound is 30 minutes
+// and a check runs several steps; an hour covers a whole check.
+const askCheckBound = time.Hour
+
+// waitsCheckBeforeAsking is how the wait for the check's verdict begins, as `show` and the delivery's wait say it.
+const waitsCheckBeforeAsking = "waits its check before asking the operator's Approve: "
+
+// verdictWaits is why an Approve is not asked yet while the check gates no merge: the check at the heads the ask would cover
+// has no verdict (the operator, 2026-10-27 23:54: "if you don't want me to approve it, don't send the approval request").
+// A pass or a failure is asked (a failure in its words: the operator may still merge it), and so is a check that has not
+// answered within askCheckBound of the heads becoming ready, which the ask says. For a group the check is the one of all its
+// heads together; for a placement change it ends only once every node it places answered (ADR 0559 §5). Under the lock.
+func (h *Holder) verdictWaits(d *Delivery) string {
+	members := h.askMembers(d)
+	c := h.checkAt(members)
+	if c.Key == checkSaidPass || c.Key == checkSaidFail {
+		return ""
+	}
+	until := h.readySince(d, members).Add(askCheckBound)
+	if !h.Now().Before(until) {
+		return ""
+	}
+	h.lookAt(until, "an Approve that waits its check, asked anyway at its bound")
+	return waitsCheckBeforeAsking + c.Words + "; asked anyway at " + until.UTC().Format("15:04") + " UTC if it has not answered"
+}
+
+// readySince is when the latest of the heads an ask would cover became ready: the bound of verdictWaits counts from it.
+func (h *Holder) readySince(d *Delivery, members []*Delivery) time.Time {
+	since := h.Now()
+	if d.Ready != nil {
+		since = d.Ready.Seen
+	}
+	for _, m := range members {
+		if m.Ready != nil && m.Ready.Seen.After(since) {
+			since = m.Ready.Seen
+		}
+	}
+	return since
+}
+
+// unansweredWords is the check's words in an ask made past askCheckBound with no verdict: it says the check has not answered.
+func unansweredWords(c checkSaid) checkSaid {
+	if c.Key == checkSaidPass || c.Key == checkSaidFail {
+		return c
+	}
+	c.Words += ", and has given none in " + strconv.Itoa(int(askCheckBound.Minutes())) + " minutes, so the operator is asked anyway"
+	return c
+}
+
 // waitsForReview is how a hold for a review begins, as the delivery's state and its mesh/delivery status say it.
 const waitsForReview = "waits for a review at "
 

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

@@ -1,7 +1,6 @@
 package main
 
 import (
-	"encoding/json"
 	"errors"
 	"sort"
 	"strings"
@@ -70,27 +69,32 @@ func TestAnApprovedPullRequestIsMergedThroughTheMergeTool(t *testing.T) {
 	}
 
 	// 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.
+	// its check" only when it passed. While the check gates no merge, the ask waits for the check's verdict (the operator,
+	// 2026-10-27 23:54: "if you don't want me to approve it, don't send the approval request"): a pass or a failure is asked,
+	// the failure in its words; with no verdict nothing is asked and the delivery says it waits its check, until the bound,
+	// past which it is asked anyway, saying so, and asked again in new words once the check passes. The verdict is the
+	// checker's verdicts bucket value as captured on the bus (mesh-delivery-checker, `<group>.<heads hash>`).
 	for _, row := range []struct {
-		name, state, failed string
-		gated               bool
-		asks                int
-		want, not           []string
+		name, state, failed, then string
+		gated                     bool
+		after                     time.Duration
+		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,
+		{name: "ungated, no verdict", asks: 0, want: []string{waitsCheckBeforeAsking + "its check has no verdict yet; asked anyway at "}},
+		{name: "ungated, running", state: "running", asks: 0, want: []string{waitsCheckBeforeAsking + "its check has no verdict yet: it is running"}},
+		{name: "ungated, running past its bound", state: "running", after: askCheckBound, asks: 1,
+			want: []string{"its check has no verdict yet: it is running", "has given none in 60 minutes, so the operator is asked anyway",
+				"Approve merges it into main at " + readyHead[:12] + " now, before its check ends"}, not: []string{"passed"}},
+		{name: "ungated, past its bound, then passed", state: "running", after: askCheckBound, then: "pass", asks: 2,
+			want: []string{"passed its check. Approve merges it into main at " + readyHead[:12] + ";"}, not: []string{"before its check ends", "anyway"}},
+		{name: "ungated, passed", state: "pass", asks: 1,
 			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"}},
+		{name: "ungated, failed", state: "fail", failed: "TestThing: wanted 1, got 2", asks: 1,
+			want: []string{"its check failed: TestThing: wanted 1, got 2", "although its check failed"}, not: []string{"passed", "anyway"}},
 	} {
 		t.Run(row.name, func(t *testing.T) {
 			v := newWorld(t)
@@ -101,23 +105,35 @@ func TestAnApprovedPullRequestIsMergedThroughTheMergeTool(t *testing.T) {
 			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)
-			}
+			head := headOf("novox/app", 7, readyHead)
 			if row.gated {
 				v.h.Checked(judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1"))
 			}
 			v.settleAll()
 			if !row.gated && row.state != "" {
-				put()
+				checkerSays(v, "g", row.state, row.failed, head)
+				v.settleAll()
+			}
+			if row.after > 0 {
+				v.later(row.after)
+				v.settleAll()
+			}
+			if row.then != "" {
+				checkerSays(v, "g", row.then, row.failed, head)
 				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)
+			if v.op.count() != row.asks || len(open) != min(row.asks, 1) {
+				t.Fatalf("open asks %v, asked %d times; wanted asked %d times", open, v.op.count(), row.asks)
+			}
+			if row.asks == 0 {
+				d := v.delivery(IDOf("novox/app", readyHead))
+				for _, w := range row.want {
+					if d.Ready == nil || !strings.Contains(d.Ready.Waits, w) {
+						t.Fatalf("the delivery's wait does not say %q: %+v", w, d.Ready)
+					}
+				}
+				return
 			}
 			e := v.op.ask(open[0]).Explanation
 			for _, w := range row.want {
@@ -146,8 +162,8 @@ func TestAnApprovedPullRequestIsMergedThroughTheMergeTool(t *testing.T) {
 	u.h.PullUpdated(pr("novox/app", 7, readyHead, "feat/x", "src/a.go"))
 	u.h.PullUpdated(pr("novox/app", 8, other, "feat/y", "src/b.go"))
 	u.settleAll()
-	if u.op.count() != 2 || u.state(IDOf("novox/app", readyHead)) != Ready {
-		t.Fatalf("ungated, two reviewed pull requests with no verdict: %d asks, %s", u.op.count(), u.state(IDOf("novox/app", readyHead)))
+	if u.op.count() != 0 || u.state(IDOf("novox/app", readyHead)) != Ready {
+		t.Fatalf("ungated, two reviewed pull requests with no verdict: %d asks, %s; wanted ready and not asked", u.op.count(), u.state(IDOf("novox/app", readyHead)))
 	}
 	if gates, _ := u.h.Status("listening")["check-gates-merge"].(bool); gates {
 		t.Fatalf("delivery_status does not say the check gates no merge: %v", u.h.Status("listening"))
@@ -157,8 +173,12 @@ func TestAnApprovedPullRequestIsMergedThroughTheMergeTool(t *testing.T) {
 		t.Fatalf("show does not say the check gates no merge: %v", shown)
 	}
 	u.h.Checked(judgedAt("novox/app", 8, other, "fail", "t1t1t1t1t1t1"))
+	checkerSays(u, "feat/y", "fail", "TestB", headOf("novox/app", 8, other))
 	u.h.PullMerged(merged("novox/app", 7, readyHead, "m7m7m7m7m7m7"))
 	u.settleAll()
+	if u.op.count() != 1 {
+		t.Fatalf("ungated, a failed verdict is asked once (the check gates no merge): %d asks", u.op.count())
+	}
 	d = u.delivery(IDOf("novox/app", other))
 	if d.State != Ready || d.Check == nil || d.Check.Passes() {
 		t.Fatalf("ungated, after a failed verdict and a trunk's move: %s, verdict %+v", d.State, d.Check)
@@ -323,13 +343,21 @@ func TestAGroupMergesEveryMemberInOnePassOnlyOnceComposedAndApproved(t *testing.
 	u.h.PullUpdated(pr("novox/mesh-controller", 1, ctlHead, "feat/x", "internal/a.go"))
 	u.h.PullUpdated(pr("novox/mesh-catalog", 2, catHead, "feat/x", "modules/app/module.json"))
 	u.settleAll()
-	if u.op.count() != 1 {
-		t.Fatalf("ungated, the group is asked %d times; wanted once", u.op.count())
+	group := []sdk.Head{headOf("novox/mesh-controller", 1, ctlHead), headOf("novox/mesh-catalog", 2, catHead)}
+	checkerSays(u, "feat/x", "running", "", group...)
+	u.settleAll()
+	// The group's check is the one of its heads together: while it runs, nothing is asked and each member says it waits.
+	if d := u.delivery(IDOf("novox/mesh-catalog", catHead)); u.op.count() != 0 || d.Ready == nil ||
+		!strings.Contains(d.Ready.Waits, waitsCheckBeforeAsking+"its check has no verdict yet: it is running") {
+		t.Fatalf("ungated, the group is asked %d times while its check runs; wanted none, and the wait said: %+v", u.op.count(), d.Ready)
 	}
-	// 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)
+	checkerSays(u, "feat/x", "pass", "", group...)
+	u.settleAll()
+	if u.op.count() != 1 {
+		t.Fatalf("ungated, the group is asked %d times once its check passed; wanted once", u.op.count())
+	}
+	if e := u.op.ask(u.op.open()[0]).Explanation; !strings.Contains(e, "2 pull requests, passed its check. Approve merges all of them into main in one pass, at the heads named;") {
+		t.Fatalf("ungated, the group's ask does not say its check passed:\n%s", e)
 	}
 	u.op.answer(u.op.open()[0], OptionApprove)
 	u.settleAll()