Merge pull request 'Ask a group member's own repo-check when the group is held for it (issue 1792)' (#424) from fix/1792-recheck-stale-member into main

2026-10-21 21:38:09 +00:00
parent 3793175894 d2faada61b
commit f1fb73e12c
3 changed files with 141 additions and 3 deletions
@@ -380,18 +380,19 @@ func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) {
 		// 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 holdOwn bool
 		var readErr error
 		for _, m := range reads {
 			if gid == "" {
 				break
 			}
-			says, _, err := h.notASuccess(m)
+			says, ownOnly, err := h.notASuccess(m)
 			if err != nil {
 				readErr = err
 				break
 			}
 			if says != "" {
-				holdMember, holdSays = m.id, says
+				holdMember, holdSays, holdOwn = m.id, says, ownOnly
 				break
 			}
 		}
@@ -421,7 +422,18 @@ func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) {
 		if readErr != nil || holdSays != "" {
 			stale()
 			h.mu.Lock()
-			defer h.mu.Unlock()
+			var own func()
+			// A member's own repository check pending or missing (a sibling's new head put it out of date, issue 1792) is
+			// asked again here, once for the head: nothing else asks it, and the composed check sets no repository check.
+			if d := h.deliveries[holdMember]; holdOwn && d != nil && d.Ready != nil {
+				own = h.askOwnAfterRefusal(d)
+			}
+			defer func() {
+				h.mu.Unlock()
+				if own != nil {
+					own()
+				}
+			}()
 			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 {
@@ -995,6 +995,12 @@ func (h *Holder) Checked(c CheckedEvent) {
 		}
 		h.moved(d, []Transition{t})
 	}
+	if d.Ready != nil && !d.Ready.OwnAskedAt.IsZero() {
+		// The own check asked for this head answered (issue 1792): one own ask is in flight at a time, so a status still
+		// pending or missing after its answer is asked again.
+		d.Ready.OwnAskedAt = time.Time{}
+		h.keep(d)
+	}
 	if d.State == Rejected && c.Trunk != "" && d.Check != nil && !sameCommit(d.Check.Trunk, c.Trunk) {
 		// Judged again against another trunk commit — the merge tool's ask of a verdict out of date, or a check asked
 		// elsewhere (novox/hq ADR 0323 §5, §7): taken as the recheck it is.
@@ -0,0 +1,120 @@
+package main
+
+import "testing"
+
+const gHk2 = "f4f4f4f4f4f4f4f4f4f4f4f4f4f4f4f4f4f4f4f4"
+
+// Issue 1792: a sibling's new head put a member's own mesh/repo-check back to pending ("out of date: judged beside ..."),
+// and nothing asked it again. The composed check passed, the group stayed held ("unasked") on the member's pending
+// status, and the operator's Approve was never asked. The group's hold on a member's own pending repository check asks
+// that check, once for the head, with no composed check (ADR 0382 §2: each member's own check stays required).
+func TestAGroupHeldBeforeItsAskOnAMembersOwnPendingRepoCheckAsksThatCheckOnce(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")
+	setChecks(w, gCtl, gHc, "success", "pending")
+	asked := len(w.ctl.checks)
+	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()
+	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 asked once", own, composed)
+	}
+	if w.op.count() != 0 {
+		t.Fatalf("the operator was asked with a required status pending: %d", w.op.count())
+	}
+}
+
+func composedPassAt(w *world, hc, hk string) {
+	g := w.h.groups["feat/g"]
+	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: hc, Trunk: gTrunk0[:12]},
+			{Owner: "novox", Repo: "mesh-catalog", Commit: hk, Trunk: gTrunk0[:12]}}})
+}
+
+// heldOnOwnPending is the group of issue 1792 after its first own ask: the member's own check asked, once.
+func heldOnOwnPending(t *testing.T) *world {
+	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()
+	setChecks(w, gCat, gHk, "success", "success")
+	setChecks(w, gCtl, gHc, "success", "pending")
+	asked := len(w.ctl.checks)
+	composedPassAt(w, gHc, gHk)
+	w.settleAll()
+	w.later(time30s)
+	w.settleAll()
+	if own, _ := ownChecks(w, asked); len(own) != 1 {
+		t.Fatalf("the first own ask: %v", own)
+	}
+	return w
+}
+
+// A second sibling push puts the member's repo-check out of date again after its own check answered: asked again.
+func TestAMembersOwnCheckIsAskedAgainWhenASecondSiblingHeadPutsItOutOfDate(t *testing.T) {
+	w := heldOnOwnPending(t)
+	w.h.Checked(judged(gCtl, 1, gHc, "pass", gTrunk0)) // its own check answered
+	setChecks(w, gCtl, gHc, "success", "success")
+	w.settleAll()
+	w.h.PullUpdated(pr(gCat, 2, gHk2, "feat/g", "modules/app/module.json"))
+	w.h.Checked(judged(gCat, 2, gHk2, "pass", gTrunk0))
+	w.settleAll()
+	setChecks(w, gCat, gHk2, "success", "success")
+	setChecks(w, gCtl, gHc, "success", "pending")
+	asked := len(w.ctl.checks)
+	composedPassAt(w, gHc, gHk2)
+	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 %v composed %v; wanted the member's own check asked again, once", own, composed)
+	}
+}
+
+// An own ask before the Approve, answered, and then a merge refused for the member's own pending status at the same head:
+// asked again (the one ask is per answer, not per head's life).
+func TestAMergeRefusedForOwnPendingAfterAnAnsweredOwnAskAsksItAgain(t *testing.T) {
+	w := heldOnOwnPending(t)
+	w.h.Checked(judged(gCtl, 1, gHc, "pass", gTrunk0))
+	setChecks(w, gCtl, gHc, "success", "success")
+	w.later(time30s)
+	w.settleAll()
+	if len(w.op.open()) != 1 {
+		t.Fatalf("the group is not asked once its statuses are green: %d", w.op.count())
+	}
+	w.forge.refuseRepo = map[string]error{gCtl: ownPending}
+	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 %v composed %v; wanted the member's own check asked again after the refusal", own, composed)
+	}
+}