diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go index 71c3fc5b..320829d6 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go @@ -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 { diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go index 627ffabe..9b6d8135 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go @@ -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. diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1792_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1792_test.go new file mode 100644 index 00000000..1c9e0852 --- /dev/null +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1792_test.go @@ -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) + } +}