Leave a delivery that needs a new head to its author for two hours, then name the author to the operator (ADR 0505 §5)

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

@@ -163,3 +163,85 @@ func TestAnyOtherRefusalIsStalledAtOnce(t *testing.T) {
 		t.Fatalf("a missing status refusal is not said at once: %+v", got)
 	}
 }
+
+// rejectedAfterItsApprove is a delivery approved and then rejected: its check failed after the ask (as in events_test.go),
+// its pull request's description naming who pushed it when pushedBy is given.
+func rejectedAfterItsApprove(t *testing.T, pushedBy string) (*world, string) {
+	t.Helper()
+	w := newWorld(t)
+	w.op.auto = false
+	p := pr("novox/app", 7, readyHead, "feat/x", "src/a.go")
+	if pushedBy != "" {
+		p.Body += "\nPushed-by: " + pushedBy
+	}
+	w.h.PullUpdated(p)
+	w.settleAll()
+	id := IDOf("novox/app", readyHead)
+	hear(t, w, ControllerSeat+".checked", judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1"))
+	if _, err := w.h.Recheck(id, "a second look", "tester"); err != nil {
+		t.Fatal(err)
+	}
+	hear(t, w, ControllerSeat+".checked", judgedAt("novox/app", 7, readyHead, "fail", "t1t1t1t1t1t1"))
+	approve(w)
+	w.h.React()
+	if w.state(id) != Rejected {
+		t.Fatalf("not rejected: %s", w.state(id))
+	}
+	return w, id
+}
+
+// ADR 0505 §5: a rejected delivery is told on its pull request at once, and reaches the operator only after two hours
+// with no new head, in words naming its author (its branch, and the session that pushed it) and needing a new head —
+// never "stop it", never "the operator's".
+func TestARejectedDeliveryIsItsAuthorsForTwoHours(t *testing.T) {
+	w, id := rejectedAfterItsApprove(t, "quiet-deliveries@shanks")
+	said := false
+	for _, c := range w.forge.comments {
+		said = said || strings.Contains(c, "not merged")
+	}
+	if !said {
+		t.Fatalf("its author was not told on the pull request at once: %v", w.forge.comments)
+	}
+	w.later(2*time.Hour - time.Minute)
+	if got := linesFor(w, id); len(got) != 0 {
+		t.Fatalf("the operator's before two hours: %+v", got)
+	}
+	w.later(2 * time.Minute)
+	got := linesFor(w, id)
+	if len(got) != 1 {
+		t.Fatalf("not the operator's after two hours with no new head: %+v", got)
+	}
+	l := got[0]
+	if l.Needs != NeedsNewHead || l.Author != "the author of feat/x (quiet-deliveries on shanks)" || l.Branch != "feat/x" {
+		t.Fatalf("the line does not name its author and the new head it needs: %+v", l)
+	}
+	for _, never := range []string{"operator's", "stop it"} {
+		if strings.Contains(l.Says+l.H2, never) {
+			t.Fatalf("the line says %q: %+v", never, l)
+		}
+	}
+	if !strings.Contains(l.Says, "needs a new head from the author of feat/x (quiet-deliveries on shanks)") {
+		t.Fatalf("the words do not say a new head is needed from its author: %q", l.Says)
+	}
+}
+
+// ADR 0505 §5: with no Pushed-by line, the branch alone names its author.
+func TestARejectedDeliveryWithNoPushedByLineIsNamedByItsBranch(t *testing.T) {
+	w, id := rejectedAfterItsApprove(t, "")
+	w.later(2*time.Hour + time.Minute)
+	if got := linesFor(w, id); len(got) != 1 || got[0].Author != "the author of feat/x" {
+		t.Fatalf("not named by its branch: %+v", got)
+	}
+}
+
+// ADR 0505 §5: a new head within the two hours ends it: the rejected head is never the operator's.
+func TestANewHeadWithinTwoHoursKeepsTheRejectedHeadFromTheOperator(t *testing.T) {
+	w, id := rejectedAfterItsApprove(t, "quiet-deliveries@shanks")
+	w.later(time.Hour)
+	w.h.PullUpdated(pr("novox/app", 7, pushed, "feat/x", "src/a.go"))
+	w.settleAll()
+	w.later(2 * time.Hour)
+	if got := linesFor(w, id); len(got) != 0 {
+		t.Fatalf("the superseded head reached the operator: %+v", got)
+	}
+}

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

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"strings"
 	"testing"
+	"time"
 
 	stdio "git.novox.be/novox/mesh-sdk/go"
 )
@@ -229,7 +230,8 @@ func TestAPushAfterTheAskEndsItAndSaysSoOnceOnThePullRequest(t *testing.T) {
 	}
 }
 
-// Issue 1022 (ADR 0421 §7): a check that failed after the ask merges nothing and is said once and as a stalled line.
+// Issue 1022 (ADR 0421 §7), as novox/hq ADR 0505 §5 changed it: a check that failed after the ask merges nothing, is
+// said once on the pull request at once, and is a stalled line only after two hours with no new head.
 func TestACheckThatFailsAfterTheAskIsNotMergedAndIsSaid(t *testing.T) {
 	w := announced(t)
 	id := IDOf("novox/app", readyHead)
@@ -252,6 +254,10 @@ func TestACheckThatFailsAfterTheAskIsNotMergedAndIsSaid(t *testing.T) {
 	if said != 1 {
 		t.Fatalf("the failure is not said once: %v", w.forge.comments)
 	}
+	if lines := w.h.Stalled(); len(lines) != 0 {
+		t.Fatalf("the failure is the operator's before its author had two hours: %+v", lines)
+	}
+	w.later(2*time.Hour + time.Minute)
 	lines := w.h.Stalled()
 	if len(lines) != 1 || lines[0].ID != id || strings.Contains(lines[0].Says, "marked") {
 		t.Fatalf("the failure is not a stalled line: %+v", lines)

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

@@ -376,6 +376,11 @@ func (h *Holder) pullUpdated(p PullEvent) (isNew bool) {
 				d.ReviewedBy, d.ReviewedAt = by, at
 				h.keep(d)
 			}
+			// So is the line naming who pushed it (ADR 0505 §5).
+			if by := PushedIn(p.Body); p.Body != "" && by != d.PushedBy {
+				d.PushedBy = by
+				h.keep(d)
+			}
 			// An `after:` line written into the description later is told about too (novox/hq ADR 0419 §2).
 			if after := AfterIn(p.Body); p.Body != "" && !slices.Equal(after, d.After) {
 				d.After = after
@@ -453,6 +458,7 @@ func (h *Holder) fillFromAnnouncement(d *Delivery, p PullEvent) {
 	d.ModuleDirs, d.ModuleDirsSaid = p.ModuleDirs, p.ModuleDirsSaid
 	d.Issues, d.After = IssuesIn(p.Title+"\n"+p.Body), AfterIn(p.Body)
 	d.ReviewedBy, d.ReviewedAt = ReviewIn(p.Body)
+	d.PushedBy = PushedIn(p.Body)
 	if p.BaseSHA != "" {
 		d.BaseSHA = p.BaseSHA
 	}
@@ -506,6 +512,7 @@ func (h *Holder) PullEdited(p PullEvent) {
 	}
 	before, _ := reviewWords(d)
 	d.ReviewedBy, d.ReviewedAt = ReviewIn(p.Body)
+	d.PushedBy = PushedIn(p.Body)
 	d.After = AfterIn(p.Body)
 	title := p.Title
 	if title == "" {
@@ -573,6 +580,7 @@ func (h *Holder) freshAnnouncement(p PullEvent, now time.Time, startedOver strin
 		PathsTruncated: p.PathsTruncated, Removed: p.Removed, ModuleDirs: p.ModuleDirs, ModuleDirsSaid: p.ModuleDirsSaid,
 		After: AfterIn(p.Body), Issues: IssuesIn(p.Title + "\n" + p.Body), Created: now, BaseSHA: p.BaseSHA}
 	d.ReviewedBy, d.ReviewedAt = ReviewIn(p.Body)
+	d.PushedBy = PushedIn(p.Body)
 	why := fmt.Sprintf("%s#%d's head announced", d.Repository, d.Number)
 	if startedOver != "" {
 		why = fmt.Sprintf("%s; %s", why, startedOver)

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

@@ -135,6 +135,9 @@ type Delivery struct {
 	// hold: no Approve is asked until it names the head (novox/hq ADR 0496, issue 1803).
 	ReviewedBy string `json:"reviewed_by,omitempty"`
 	ReviewedAt string `json:"reviewed_at,omitempty"`
+	// PushedBy is the session that pushed its head, as its pull request's description names it in a line
+	// `Pushed-by: <who@node>` (playbook 07): named with its branch when a new head is needed (novox/hq ADR 0505 §5).
+	PushedBy string `json:"pushed_by,omitempty"`
 	// FreshWanted is whether a passing verdict that arrived out of date is to have its check asked against the
 	// trunk's head, before the operator is asked (novox/hq ADR 0421 §3, issue 1022). Cleared when it is asked.
 	FreshWanted bool `json:"fresh_wanted,omitempty"`
@@ -498,6 +501,31 @@ func ReviewIn(body string) (who, sha string) {
 	return who, sha
 }
 
+// pushedLine is the line naming the session that pushed a pull request's head: `Pushed-by: <who@node>` (ADR 0505 §5).
+var pushedLine = regexp.MustCompile(`(?im)^\s*Pushed-by:\s*([A-Za-z0-9_.\-]+(?:@[A-Za-z0-9_.\-]+)?)\s*$`)
+
+// PushedIn reads it: the last such line, or "" when there is none.
+func PushedIn(body string) string {
+	who := ""
+	for _, m := range pushedLine.FindAllStringSubmatch(body, -1) {
+		who = m[1]
+	}
+	return who
+}
+
+// authorOf is a delivery's author as the operator reads it: "the author of <branch>", and the session that pushed it
+// when its description names one ("who on node": the router refuses an @ in its words).
+func authorOf(d *Delivery) string {
+	who := "the author of " + d.Branch
+	if d.Branch == "" {
+		who = "its author"
+	}
+	if d.PushedBy != "" {
+		who += " (" + strings.Replace(d.PushedBy, "@", " on ", 1) + ")"
+	}
+	return who
+}
+
 // reviewWords is what the delivery's review record says of its head, for the ask: reviewed by whom, or, first, that it
 // is not reviewed at this head (the ask is held then, ADR 0496). ok is whether it is reviewed at this head.
 func reviewWords(d *Delivery) (words string, ok bool) {

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

@@ -480,10 +480,17 @@ func (h *Holder) readyLines(now time.Time) []StalledLine {
 				Bound: headHeardBound.String(), H2: "none: a later announcement of the pull request's head (the merge tool announces it again, the forge's announcer restarts, or a new push) or an edit of its description; a recheck does not fill it — the operator's",
 				Says: fmt.Sprintf("%s#%d passed its checks at %s and waits for %s", d.Repository, d.Number, shortOf(r.Head, 12), waitsForHead)})
 		case r.Failed != "" && d.State == Rejected:
-			out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State, For: now.Sub(r.Seen).Round(time.Second).String(),
-				Bound: "0s", H2: "none: a new head, or a recheck that passes — the operator's",
-				Says: fmt.Sprintf("%s#%d passed its checks at %s and is not merged: %s", d.Repository, d.Number,
-					shortOf(r.Head, 12), r.Failed)})
+			// Its cure is its author's new head: listed here at once (`show`), told on the pull request at once, and the
+			// operator's only once no new head came within the author's bound (novox/hq ADR 0505 §5).
+			if now.Sub(d.Since) < h.authorBound() {
+				continue
+			}
+			out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State, For: now.Sub(d.Since).Round(time.Second).String(),
+				Bound: h.authorBound().String(), H2: "none: a new head from its author, or a recheck that passes",
+				Needs: NeedsNewHead, Author: authorOf(d), Branch: d.Branch,
+				Says: fmt.Sprintf("%s#%d passed its checks at %s and is not merged: %s. It needs a new head from %s, "+
+					"and none came for %s", d.Repository, d.Number, shortOf(r.Head, 12), r.Failed, authorOf(d),
+					now.Sub(d.Since).Round(time.Minute))})
 		case r.Refused != "":
 			out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State, For: now.Sub(r.Seen).Round(time.Second).String(),
 				Bound: "0s", H2: "none: a new head, which asks the operator again — the operator's",

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

@@ -233,8 +233,16 @@ type StalledLine struct {
 	Says  string `json:"says"`
 	// Severity is "urgent" for a group that is held or half merged (issue 1275); empty otherwise.
 	Severity string `json:"severity,omitempty"`
+	// Needs is NeedsNewHead for a delivery whose cure is its author's new head (novox/hq ADR 0505 §5), with Author
+	// naming that author as the operator reads it and Branch its pull request's branch; empty otherwise.
+	Needs  string `json:"needs,omitempty"`
+	Author string `json:"author,omitempty"`
+	Branch string `json:"branch,omitempty"`
 }
 
+// NeedsNewHead is what a stalled line needs when its cure is its author's new head (ADR 0505 §5).
+const NeedsNewHead = "new-head"
+
 // Stalled is the `stalled` verb: what the controller's self-check raises as `delivery.<id>.stalled`.
 func (h *Holder) Stalled() []StalledLine {
 	h.mu.Lock()