diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/events.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/events.go index 1cd053de7..18bdfef88 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/events.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/events.go @@ -81,7 +81,7 @@ func (h *Holder) CommitStatus(e ForgeStatusEvent) { head = sha } if sameCommit(head, e.SHA) { - h.mergeAgainNow(d) // its approved head's check turned success on the forge: the merge is asked again at once + h.causeChanged(d) // its approved head's check turned success on the forge: the merge is asked again at once, the count anew } } } diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1348_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1348_test.go index fc25f9384..fdcebc717 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1348_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1348_test.go @@ -224,3 +224,23 @@ func TestARefusalThatRepeatsWithNoTrunkMoveIsNotAskedAgainWithoutEnd(t *testing. t.Fatal("a trunk's move did not let the merge be asked again") } } + +// 1785: a verdict, a composed check or a status of the same heads that changes what the refusal waited on starts the count of +// the same words again, so an approved merge whose cause cleared is asked again without a second Approve; a status that is no +// success changes nothing. +func TestANewCauseOfTheSameHeadsStartsTheCountOfARepeatedRefusalAgain(t *testing.T) { + w := answeredFirst(t) + d := w.h.deliveries[IDOf(gCat, gHk)] + d.Ready.Repeats, d.Ready.RepeatTrunk = maxRepeatedRefusals, d.TrunkNow + d.Ready.LastTry = w.now + owner, repo, _ := strings.Cut(gCat, "/") + w.h.CommitStatus(ForgeStatusEvent{Owner: owner, Repo: repo, SHA: gHk, Context: "mesh/check", State: "pending"}) + if d.Ready.Repeats != maxRepeatedRefusals || d.Ready.LastTry.IsZero() { + t.Fatalf("a status that is no success changed the refusal: %d repeats", d.Ready.Repeats) + } + w.h.CommitStatus(ForgeStatusEvent{Owner: owner, Repo: repo, SHA: gHk, Context: "mesh/check", State: "success"}) + if d.Ready.Repeats != 0 || !d.Ready.LastTry.IsZero() { + t.Fatalf("the check turned success on the same head: %d repeats, last try %v; the merge is asked again and the count starts anew", + d.Ready.Repeats, d.Ready.LastTry) + } +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/onestatus.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/onestatus.go index 554ad13f3..9afb0c564 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/onestatus.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/onestatus.go @@ -185,7 +185,7 @@ func (h *Holder) takeCheckStatus(repository, sha, state, desc string) { return // past its verdict: the merge reads the forge's status itself } d.Check, d.AskedAgain, d.CheckAsk = v, nil, nil - h.mergeAgainNow(d) + h.causeChanged(d) if why := d.outOfDate(); why != "" { h.Logf("[mesh-delivery] %s: its mesh/check is %s; the checker checks it again", d.ID, why) } @@ -299,7 +299,7 @@ func (h *Holder) composeFromChecker(g *Group, members []*Delivery, now time.Time h.keepGroup(g) if c.Verdict != "" { for _, d := range members { - h.mergeAgainNow(d) + h.causeChanged(d) // a composed check that differs from the last: what a refusal waited on changed (issue 1785) } h.Logf("[mesh-delivery] group %s: %s — %s", g.ID, c.Verdict, c.Summary) } diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go index 7ce09b063..18749f9f6 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go @@ -323,6 +323,16 @@ func (h *Holder) mergeAgainNow(d *Delivery) { } } +// causeChanged takes a verdict, a composed check or a status of the same heads that decides whether the merge may go: what the +// refusal waited on changed, so the count of the same words starts again and the merge is asked at once (issue 1785); only a +// refusal that truly repeats, with nothing new behind it, stops the asking (issue 1106). Under the lock. +func (h *Holder) causeChanged(d *Delivery) { + if d != nil && d.Ready != nil && d.Ready.Merged == nil { + d.Ready.Repeats = 0 + } + h.mergeAgainNow(d) +} + // maxRepeatedRefusals is how many refusals in a row, with the same words and the trunk not moved, end the asking again: a // refusal that repeats with no trunk move is not a trunk's move, and the check asked after each only brings the same words. const maxRepeatedRefusals = 3