Say a refusal the gate is already answering as stalled only once its fresh check did not cure it (ADR 0505 §4)

2026-10-22 14:15:49 +02:00
parent 5ddc3ff312
commit f1fbd91f0d
4 changed files with 151 additions and 20 deletions
@@ -1,8 +1,10 @@
 package main

 import (
+	"encoding/json"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 )
@@ -65,3 +67,99 @@ func TestTheLastMeasuredCheckTimeIsTheNewestVerdictsOfItsRepository(t *testing.T
 		t.Fatalf("no verdict: %s", got)
 	}
 }
+
+// refusedForAMovedTrunk is a lone delivery approved and refused by the merge tool because its trunk moved and its check
+// was asked again (gitea merge.ts:147, errAskedAgain).
+func refusedForAMovedTrunk(t *testing.T) (*world, string) {
+	t.Helper()
+	w := announced(t)
+	id := IDOf("novox/app", readyHead)
+	hear(t, w, ControllerSeat+".checked", judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1"))
+	w.forge.refuse = errAskedAgain
+	approve(w)
+	if len(w.forge.merges) != 1 || w.delivery(id).Ready.RefusedSince == nil {
+		t.Fatalf("the merge was not asked and refused: %v %+v", w.forge.merges, w.delivery(id).Ready)
+	}
+	return w, id
+}
+
+func linesFor(w *world, id string) []StalledLine {
+	var out []StalledLine
+	for _, l := range w.h.Stalled() {
+		if l.ID == id {
+			out = append(out, l)
+		}
+	}
+	return out
+}
+
+// ADR 0505 §4: a refusal because the trunk moved, whose check the gate asked again, is in the gate's own list at once
+// but is no stalled line while its check is unanswered; once that check passed and the merge is refused again, it is.
+func TestATrunkMovedRefusalIsStalledOnlyOnceItsFreshCheckAnsweredAndTheMergeIsRefusedAgain(t *testing.T) {
+	w, id := refusedForAMovedTrunk(t)
+	if got := linesFor(w, id); len(got) != 0 {
+		t.Fatalf("said as stalled while its fresh check runs: %+v", got)
+	}
+	shown, err := w.h.Show(id)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if raw, _ := json.Marshal(shown); !strings.Contains(string(raw), "is not merged: its verdict was judged against trunk") {
+		t.Fatalf("the gate's own list does not show the refusal at once: %s", raw)
+	}
+	w.later(5 * time.Minute)
+	if got := linesFor(w, id); len(got) != 0 {
+		t.Fatalf("said as stalled at five minutes, its check unanswered: %+v", got)
+	}
+	// As on the live mesh (events_test.go): the trunk moved, the head is announced again, its fresh verdict passes
+	// against the new trunk, and the merge is asked and refused again, the trunk having moved once more.
+	w.h.PullMerged(merged("novox/app", 9, "9999999999999999", "t2t2t2t2t2t2"))
+	w.h.PullUpdated(pr("novox/app", 7, readyHead, "feat/x", "src/a.go"))
+	w.later(time.Second)
+	hear(t, w, ControllerSeat+".checked", judgedAt("novox/app", 7, readyHead, "pass", "t2t2t2t2t2t2"))
+	w.settleAll()
+	if len(w.forge.merges) != 2 {
+		t.Fatalf("the fresh verdict did not ask the merge again: %v", w.forge.merges)
+	}
+	if got := linesFor(w, id); len(got) != 1 {
+		t.Fatalf("its check answered and the merge refused again, yet no stalled line: %+v", got)
+	}
+}
+
+// ADR 0505 §4: with no answer, the refusal is said at twice the repository's last check time, at least ten minutes.
+func TestATrunkMovedRefusalIsStalledAtTwiceTheLastCheckTimeAtLeastTheFloor(t *testing.T) {
+	w, id := refusedForAMovedTrunk(t)
+	w.later(9 * time.Minute)
+	if got := linesFor(w, id); len(got) != 0 {
+		t.Fatalf("said before the floor: %+v", got)
+	}
+	w.later(2 * time.Minute)
+	if got := linesFor(w, id); len(got) != 1 {
+		t.Fatalf("not said past the floor of ten minutes: %+v", got)
+	}
+
+	w, id = refusedForAMovedTrunk(t)
+	w.h.mu.Lock()
+	w.h.deliveries[id].Check.Took = "7m0s" // its repository's last check ran seven minutes: the bound is fourteen
+	w.h.mu.Unlock()
+	w.later(11 * time.Minute)
+	if got := linesFor(w, id); len(got) != 0 {
+		t.Fatalf("said at eleven minutes, under twice the last check time: %+v", got)
+	}
+	w.later(4 * time.Minute)
+	if got := linesFor(w, id); len(got) != 1 {
+		t.Fatalf("not said past twice the last check time: %+v", got)
+	}
+}
+
+// ADR 0505 §4: any other refusal is said at once, as issue 1115 has it.
+func TestAnyOtherRefusalIsStalledAtOnce(t *testing.T) {
+	w := announced(t)
+	id := IDOf("novox/app", readyHead)
+	hear(t, w, ControllerSeat+".checked", judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1"))
+	w.forge.refuse = refusedWords("novox/app#7")
+	approve(w)
+	if got := linesFor(w, id); len(got) != 1 {
+		t.Fatalf("a missing status refusal is not said at once: %+v", got)
+	}
+}
@@ -249,8 +249,9 @@ func TestAMemberBothRefusedAndHeldForAStatusIsOneStalledLine(t *testing.T) {
 	}
 }

-// 1275/AC6: a member refused because its verdict is out of date (the trunk moved; its fresh check runs) is a warning at
-// once, and urgent only when the group is not merged within ten minutes; the pass stops at the refusal all the same.
+// 1275/AC6, as novox/hq ADR 0505 §4 changed it: a member refused because its verdict is out of date (the trunk moved; its
+// fresh check runs) is no stalled line while the gate answers it, and urgent once ten minutes passed with the group not
+// merged; the pass stops at the refusal all the same.
 func TestAnOutOfDateRefusalIsAWarningAtOnceAndUrgentAfterTenMinutes(t *testing.T) {
 	w := approvedGroup(t)
 	allGreen(w)
@@ -267,12 +268,12 @@ func TestAnOutOfDateRefusalIsAWarningAtOnceAndUrgentAfterTenMinutes(t *testing.T
 		}
 		return out
 	}
-	if got := sev(); len(got) != 1 || got[0] != "warning" {
-		t.Fatalf("an out of date refusal is %v at once; wanted one warning", got)
+	if got := sev(); len(got) != 0 {
+		t.Fatalf("an out of date refusal is %v at once; wanted no line while it is answered", got)
 	}
 	w.later(9 * time.Minute)
-	if got := sev(); len(got) != 1 || got[0] != "warning" {
-		t.Fatalf("at nine minutes it is %v; wanted still a warning", got)
+	if got := sev(); len(got) != 0 {
+		t.Fatalf("at nine minutes it is %v; wanted still no line", got)
 	}
 	w.later(2 * time.Minute)
 	if got := sev(); len(got) != 1 || got[0] != "urgent" {
@@ -300,8 +301,8 @@ func severities(w *world) []string {
 	return out
 }

-// 1275/AC6 (b): the same out of date refusal on a LATER member, after the earlier one merged, is a warning at once and
-// urgent after ten minutes: the half merged group does not say it urgent at once.
+// 1275/AC6 (b), as ADR 0505 §4 changed it: the same out of date refusal on a LATER member, after the earlier one merged,
+// is no line at once and urgent after ten minutes: the half merged group does not say it at once.
 func TestAnOutOfDateRefusalOnALaterMemberIsAWarningThenUrgent(t *testing.T) {
 	w := approvedGroup(t)
 	allGreen(w)
@@ -310,8 +311,8 @@ func TestAnOutOfDateRefusalOnALaterMemberIsAWarningThenUrgent(t *testing.T) {
 	w.settleAll()
 	w.h.PullMerged(merged(gCat, 2, gHk, gT1))
 	w.settleAll()
-	if got := severities(w); len(got) != 1 || got[0] != "warning" {
-		t.Fatalf("a later member's out of date refusal is %v at once; wanted one warning", got)
+	if got := severities(w); len(got) != 0 {
+		t.Fatalf("a later member's out of date refusal is %v at once; wanted no line while it is answered", got)
 	}
 	w.later(11 * time.Minute)
 	if got := severities(w); len(got) != 1 || got[0] != "urgent" {
@@ -371,8 +372,9 @@ var mergeToolRefusals = []struct {
 		"Nothing was merged", false},
 }

-// 1275/AC6 (e): only the merge tool's two out of date refusals (:121, :147) are warnings; the one saying its check could
-// not be asked (:144) and every other is urgent at once.
+// 1275/AC6 (e), as ADR 0505 §4 changed it: only the merge tool's two out of date refusals (:121, :147) are answered by
+// the gate and said as no line at once; the one saying its check could not be asked (:144) and every other is urgent at
+// once.
 func TestOnlyTheMergeToolsTwoOutOfDateRefusalsAreWarnings(t *testing.T) {
 	for _, c := range mergeToolRefusals {
 		if got := outOfDateRefusal(c.words); got != c.outOfDated {
@@ -383,12 +385,12 @@ func TestOnlyTheMergeToolsTwoOutOfDateRefusalsAreWarnings(t *testing.T) {
 		w.forge.refuse = errors.New("the forge refused gitea_merge_pull_request: " + c.words)
 		w.op.answer(w.op.open()[0], OptionApprove)
 		w.settleAll()
-		want := "urgent"
-		if c.outOfDated {
-			want = "warning"
+		got := severities(w)
+		if c.outOfDated && len(got) != 0 {
+			t.Errorf("%s: said %v at once, wanted no line while it is answered", c.line, got)
 		}
-		if got := severities(w); len(got) != 1 || got[0] != want {
-			t.Errorf("%s: severity %v at once, wanted %s", c.line, got, want)
+		if !c.outOfDated && (len(got) != 1 || got[0] != "urgent") {
+			t.Errorf("%s: severity %v at once, wanted urgent", c.line, got)
 		}
 	}
 }
@@ -284,9 +284,14 @@ func (h *Holder) ForgeRefused(f sdk.Forge) {
 	r, now := d.Ready, h.Now()
 	r.merging, r.refusedNoted = false, false
 	answer := oneLine(f.Words)
-	if r.RefusedSince == nil {
-		r.RefusedSince = &now
+	var on time.Time
+	if d.Check != nil {
+		on = d.Check.At
 	}
+	if r.RefusedSince == nil {
+		r.RefusedSince, r.FirstRefusedOn = &now, on
+	}
+	r.LastRefusedOn = on
 	r.noteRefusal(d, answer)
 	if !outOfDateRefusal(answer) {
 		r.Urgent = true // never a warning again for this head
@@ -35,7 +35,9 @@ import (
 // delivery (Ready), said in its log, and the merge's transitions name the Approve as their warrant.

 // readyRefusedFor is how long the merge tool may refuse an approved head before it is said as stalled: none (issue 1115) —
-// a refusal is said at once, and the merge is asked again at each event that can change it, with no timer.
+// a refusal is said at once, and the merge is asked again at each event that can change it, with no timer. A refusal the
+// gate is already answering (its trunk moved and its check was asked again) is said only once that check answered and
+// the merge is still refused, or twice the repository's last check time passed (novox/hq ADR 0505 §4, `answering`).
 const readyRefusedFor time.Duration = 0

 // groupRefusedFor is how long the merge tool may refuse a closed group's member before it is said as stalled: none (issue 1115).
@@ -92,6 +94,11 @@ type ReadyRun struct {
 	RepeatTrunk string `json:"repeat_trunk,omitempty"`
 	// RefusedSince is when the merge tool began refusing it without a break.
 	RefusedSince *time.Time `json:"refused_since,omitempty"`
+	// FirstRefusedOn and LastRefusedOn are when the verdict the merge was asked on was heard, at the first refusal of
+	// the run of refusals and at the last: a later one than the first is a fresh check answered and refused again
+	// (novox/hq ADR 0505 §4).
+	FirstRefusedOn time.Time `json:"first_refused_on,omitzero"`
+	LastRefusedOn  time.Time `json:"last_refused_on,omitzero"`
 	// Merged is when the merge tool merged it.
 	Merged *time.Time `json:"merged,omitempty"`
 	// Failed is why it is not merged and will not be on this head: its check failed. Said once on the pull request.
@@ -482,6 +489,9 @@ func (h *Holder) readyLines(now time.Time) []StalledLine {
 				Bound: "0s", H2: "none: a new head, which asks the operator again — the operator's",
 				Says: fmt.Sprintf("%s#%d at %s is not merged: the operator's answer %s", d.Repository,
 					d.Number, shortOf(r.Head, 12), r.Refused)})
+		case h.answering(d, now):
+			// The trunk moved and its check was asked again: the gate is answering it, so it is no stall yet (ADR 0505
+			// §4). `show` lists the refusal meanwhile.
 		case r.RefusedSince != nil && d.Group != "" && h.groupClosed(d.Group) && now.Sub(*r.RefusedSince) >= groupRefusedFor:
 			// A closed group's member (novox/hq ADR 0418 §3, issue 1082): its sibling merged, and its own merge is refused
 			// again after each composed check. Said after the merge window, with the group and the tool's last answer.
@@ -542,6 +552,22 @@ func neededOf(d *Delivery) string {
 	return ". Needed: the refusal's cause is met (the merge is asked again at each verdict, status or Approve), or the operator decides."
 }

+// answering is whether an approved head the merge tool refused is one the gate is already answering, and so not said as
+// stalled yet (novox/hq ADR 0505 §4): refused only because its trunk moved, its check asked again, and neither has that
+// check answered with the merge refused again after it, nor has twice the repository's last check time (at least the
+// floor) passed since the refusals began. Any other refusal of this head, and a refusal repeated past its limit, is said
+// at once (r.Urgent). Under the lock.
+func (h *Holder) answering(d *Delivery, now time.Time) bool {
+	r := d.Ready
+	if r == nil || r.RefusedSince == nil || r.Urgent || !outOfDateRefusal(r.Answer) {
+		return false
+	}
+	if r.LastRefusedOn.After(r.FirstRefusedOn) {
+		return false // its fresh check answered, and the merge was refused again on it
+	}
+	return now.Sub(*r.RefusedSince) < h.answeredRefusalBound(d.Repository)
+}
+
 // outOfDateFor is how long a refusal for an out of date verdict is a warning before it is urgent (issue 1275, AC6).
 const outOfDateFor = 10 * time.Minute