mesh-delivery: say a head whose merge check started and never answered (issue 438)

The watch for unchecked heads counted any merge-check status as checked, pending included, so a head left with a pending mesh/merge-gate, or never given mesh/repo-check, waited unable to merge with nothing saying so. gitea_open_pulls now gives each head status with its state and time, and the watch says a head whose required check stays pending or unset past the delivery table's bound for a check asked and not answered.
2026-10-11 02:16:34 +02:00
parent d856892ced
commit 9d766b675d
6 changed files with 292 additions and 28 deletions
@@ -18,6 +18,9 @@ Every new head of an open pull request is announced as `pull.updated`, once, and
  `mesh-delivery` module asks it every five minutes, and a head on a base requiring `mesh/merge-gate` or
  `mesh/repo-check` with neither for ten minutes since it first saw it so is a delivery held past its bound
  (`delivery.<id>.stalled`, state `unannounced`), which the controller raises like any other (issue 347).
+  Each of the head's statuses comes with its state and when it was set, so a head whose check started and never
+  answered — a required check pending, or never set beside one that is — is said the same way once that has lasted
+  the delivery table's bound for a check asked and not answered, an hour (issue 438).
 - **How to see it work.** A pull request opened more than a minute after its branch was pushed — this fix's own
  was one, and the announcer it fixes never saw it — has `mesh/merge-gate` pending within a pass of the next sweep
  at the latest, and `mesh-controller.conditions` names none for it.
@@ -81,6 +81,16 @@ export interface OpenPull {
  required: string[];
  /** The contexts the head has a status of, in any state. */
  statuses: string[];
+  /** The head's statuses by context, the newest of each, with its state (novox/hq issue 438): what tells a check
+   *  that started and never answered — pending — from one that answered. */
+  checks: Record<string, HeadCheck>;
+}
+
+/** One status on a head, the newest of its context: its state, what it said, and when it was set. */
+export interface HeadCheck {
+  state: string;
+  description?: string;
+  updated_at?: string;
 }

 /** A commit status, as the forge keeps it: what a pull request shows beside its head commit. */
@@ -323,9 +333,10 @@ export class GiteaClient {
      if (pull.state !== "open" || !pull.head_sha) continue;
      if (!rules.has(full)) rules.set(full, await this.branchProtections(found.owner, found.name).catch(() => undefined));
      const rule = rules.get(full)?.find((p) => (p.rule_name ?? p.branch_name) === (pull.base || "main"));
-      const statuses = Object.keys(await this.commitStatuses(found.owner, found.name, pull.head_sha));
+      const checks = await this.headChecks(found.owner, found.name, pull.head_sha);
      out.push({ owner: found.owner, repo: found.name, number: pull.number, base: pull.base ?? "", head_sha: pull.head_sha,
-        html_url: pull.html_url, required: rule?.enable_status_check ? (rule.status_check_contexts ?? []) : [], statuses });
+        html_url: pull.html_url, required: rule?.enable_status_check ? (rule.status_check_contexts ?? []) : [],
+        statuses: Object.keys(checks), checks });
    }
    return out;
  }
@@ -441,6 +452,19 @@ export class GiteaClient {
    return out;
  }

+  /** A head's statuses by context, the newest of each, with state, description and time (novox/hq issue 438):
+   *  commitStatuses' read, keeping what a watch needs to say a check pending and since when. */
+  async headChecks(owner: string, repo: string, sha: string): Promise<Record<string, HeadCheck>> {
+    const raw = await this.request<any>(`${repoPath(owner, repo)}/commits/${commitSegment(sha)}/status`);
+    const out: Record<string, HeadCheck> = {};
+    for (const s of raw?.statuses ?? []) {
+      if (!s?.context || out[s.context]) continue;
+      out[s.context] = { state: String(s.status ?? s.state ?? ""), description: String(s.description ?? ""),
+        updated_at: String(s.updated_at ?? s.created_at ?? "") };
+    }
+    return out;
+  }
+
  /** A commit's statuses whole, as the forge combines them for a ref (a sha, or the start of one): the commit
   *  it resolved to and every status with its description, link, times and setter (novox/hq ADR 0239). */
  async combinedStatus(owner: string, repo: string, ref: string): Promise<{ sha: string; state: string; statuses: unknown[] }> {
@@ -54,7 +54,8 @@ test("every open pull request is read with what its base requires and what its h
      return send([{ rule_name: "main", enable_status_check: true, status_check_contexts: ["mesh/merge-gate", "mesh/repo-check"] }]);
    }
    if (url.includes("/commits/aaa1111/status")) return send({ statuses: [] });
-    if (url.includes("/commits/bbb2222/status")) return send({ statuses: [{ context: "mesh/merge-gate", status: "pending" }] });
+    if (url.includes("/commits/bbb2222/status")) return send({ statuses: [{ context: "mesh/merge-gate", status: "pending",
+      description: "the mesh is mapping this head onto its module graph", updated_at: "2026-10-11T00:11:39Z" }] });
    res.statusCode = 404;
    res.end("{}");
  });
@@ -66,6 +67,9 @@ test("every open pull request is read with what its base requires and what its h
      ["novox", "hq", 243, "main", "aaa1111", ["mesh/repo-check"], []],
      ["novox", "mesh-sdk", 14, "main", "bbb2222", ["mesh/merge-gate", "mesh/repo-check"], ["mesh/merge-gate"]],
    ]);
+    // Each status with its state and time (novox/hq issue 438): a check pending is told from one that answered.
+    assert.deepEqual(open.map((p) => p.checks), [{}, { "mesh/merge-gate": { state: "pending",
+      description: "the mesh is mapping this head onto its module graph", updated_at: "2026-10-11T00:11:39Z" } }]);
  } finally {
    server.close();
  }
@@ -617,7 +617,7 @@ function allGiteaTools(gitea: GiteaClient): ToolDefinition[] {
    },
    {
      name: "gitea_open_pulls",
-      description: "Every open pull request of every repository, from the forge itself: its head, its base, the statuses the base's protection requires and the ones its head has, in any state. What mesh-delivery reads to say a pull request the mesh was never asked to check (novox/hq issue 347). Reads only.",
+      description: "Every open pull request of every repository, from the forge itself: its head, its base, the statuses the base's protection requires and the ones its head has, each with its state, description and when it was set. What mesh-delivery reads to say a pull request the mesh was never asked to check (novox/hq issue 347), or whose check started and never answered (issue 438). Reads only.",
      input: {},
      run: async () => ({ pulls: await gitea.openPulls() }),
    },
@@ -0,0 +1,147 @@
+package main
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+	"time"
+)
+
+// openPullsOf reads gitea_open_pulls' answer as the forge module gives it, so the tests read the real shape: the
+// pull requests below are mesh-controller #212's and #211's heads and mesh-catalog #194's, as the forge said them on
+// 2026-10-11 (novox/hq issue 438), with only their numbers and shas as they were.
+func openPullsOf(t *testing.T, answer string) []OpenPull {
+	t.Helper()
+	var open struct {
+		Pulls []OpenPull `json:"pulls"`
+	}
+	if err := json.Unmarshal([]byte(answer), &open); err != nil {
+		t.Fatal(err)
+	}
+	return open.Pulls
+}
+
+// A head the mesh's check started on and never answered: mesh/merge-gate pending since the controller began mapping
+// it, mesh/repo-check never set. Its base requires both.
+const startedNeverAnswered = `{"pulls": [{
+  "base": "main", "head_sha": "c11222026a3b26a0cc4b277119e1ea059b82a36b",
+  "html_url": "http://127.0.0.1:20000/novox/mesh-controller/pulls/212", "number": 212,
+  "owner": "novox", "repo": "mesh-controller",
+  "required": ["mesh/merge-gate", "mesh/repo-check"],
+  "statuses": ["mesh/merge-gate", "mesh/delivery"],
+  "checks": {
+    "mesh/delivery": {"state": "pending", "description": "checking", "updated_at": "2026-10-11T00:11:46Z"},
+    "mesh/merge-gate": {"state": "pending", "description": "the mesh is mapping this head onto its module graph",
+      "updated_at": "2026-10-11T00:11:39Z"}
+  }
+}]}`
+
+// **A head whose merge check started and never answered is said** (novox/hq issue 438): pending, or missing a check
+// its base requires, for as long as the delivery table gives a check asked and not answered (Bounds[Proposed]) since
+// this owner first saw it so, it is a stalled line naming each check that did not answer and since when. Before,
+// any merge-check status counted as checked, pending included, and such a head was never said.
+func TestAHeadWhoseCheckStartedAndNeverAnsweredIsSaid(t *testing.T) {
+	w := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), time.Date(2026, 10, 11, 0, 12, 5, 0, time.UTC))
+	w.forge.open = openPullsOf(t, startedNeverAnswered)
+	w.h.SeeOpenPulls()
+	w.later(Bounds[Proposed].For - time.Minute)
+	w.h.SeeOpenPulls()
+	if lines := w.h.Stalled(); len(lines) != 0 {
+		t.Fatalf("a head still within the bound of a check asked and not answered is said: %+v", lines)
+	}
+	w.later(2 * time.Minute)
+	lines := w.h.Stalled()
+	if len(lines) != 1 {
+		t.Fatalf("a head whose check started and never answered is not said: %+v", lines)
+	}
+	l := lines[0]
+	if l.ID != IDOf("novox/mesh-controller", "c11222026a3b26a0cc4b277119e1ea059b82a36b") || l.Number != 212 ||
+		l.State != Unannounced || l.Bound != Bounds[Proposed].For.String() || l.For != "1h1m0s" {
+		t.Fatalf("the line is not this head's, past the table's bound: %+v", l)
+	}
+	for _, want := range []string{"mesh/merge-gate", "pending since 2026-10-11T00:11:39Z", "mesh/repo-check", "never set",
+		"never answered"} {
+		if !strings.Contains(l.Says, want) {
+			t.Fatalf("the line does not say %q: %s", want, l.Says)
+		}
+	}
+	if strings.Contains(l.Says, "mesh/delivery") {
+		t.Fatalf("the line names a status that is not the merge check: %s", l.Says)
+	}
+
+	// Announced at last: a delivery of its own, with its own bound — no longer this line.
+	w.h.PullUpdated(pr("novox/mesh-controller", 212, "c11222026a3b26a0cc4b277119e1ea059b82a36b", "fix/402"))
+	w.h.SeeOpenPulls()
+	for _, l := range w.h.Stalled() {
+		if l.State == Unannounced {
+			t.Fatalf("an announced head is still said unannounced: %+v", l)
+		}
+	}
+}
+
+// A check asked again — its pending status set anew, as a recheck or a new announcement does — starts the bound again:
+// the head is said only once that ask too has gone unanswered past it.
+func TestACheckAskedAgainStartsItsBoundAgain(t *testing.T) {
+	w := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), time.Date(2026, 10, 11, 0, 12, 5, 0, time.UTC))
+	w.forge.open = openPullsOf(t, startedNeverAnswered)
+	w.h.SeeOpenPulls()
+	w.later(50 * time.Minute)
+	again := openPullsOf(t, strings.Replace(startedNeverAnswered, "2026-10-11T00:11:39Z", "2026-10-11T01:02:00Z", 1))
+	w.forge.open = again
+	w.h.SeeOpenPulls()
+	w.later(20 * time.Minute)
+	w.h.SeeOpenPulls()
+	if lines := w.h.Stalled(); len(lines) != 0 {
+		t.Fatalf("a check asked again twenty minutes ago is said on the first ask's time: %+v", lines)
+	}
+	w.later(41 * time.Minute)
+	if lines := w.h.Stalled(); len(lines) != 1 || !strings.Contains(lines[0].Says, "pending since 2026-10-11T01:02:00Z") {
+		t.Fatalf("a check asked again and not answered past the bound is not said: %+v", lines)
+	}
+}
+
+// **A head whose required checks all answered is not said**, whatever they answered: a failure is a verdict, and the
+// pull request says it itself. Nor is one whose only pending check is one its base does not require: hq requires
+// mesh/repo-check alone.
+func TestAHeadWhoseRequiredChecksAnsweredIsNotSaid(t *testing.T) {
+	w := newWorldOver(t, newMemStore(), newFakeController(), newFakeForge(), time.Date(2026, 10, 11, 0, 12, 5, 0, time.UTC))
+	w.forge.open = openPullsOf(t, `{"pulls": [{
+  "base": "main", "head_sha": "99df6b999bad77c51dfdd13470cf57b1f38e8c24", "number": 194, "owner": "novox", "repo": "mesh-catalog",
+  "required": ["mesh/merge-gate", "mesh/repo-check"],
+  "statuses": ["mesh/merge-gate", "mesh/repo-check", "mesh/delivery"],
+  "checks": {
+    "mesh/delivery": {"state": "failure", "description": "rejected", "updated_at": "2026-10-11T00:05:37Z"},
+    "mesh/merge-gate": {"state": "success", "description": "pass: builds mounts → shanks; no bus step; every machine composes with the change as it did without (4 of 4 compose)", "updated_at": "2026-10-11T00:05:28Z"},
+    "mesh/repo-check": {"state": "failure", "description": "fail: its merge-check.sh failed", "updated_at": "2026-10-11T00:05:28Z"}
+  }
+}, {
+  "base": "main", "head_sha": "11b719fd5cdec43970e513a10a44a558042599e6", "number": 308, "owner": "novox", "repo": "hq",
+  "required": ["mesh/repo-check"],
+  "statuses": ["mesh/merge-gate", "mesh/repo-check", "mesh/delivery"],
+  "checks": {
+    "mesh/merge-gate": {"state": "pending", "description": "the mesh is mapping this head onto its module graph", "updated_at": "2026-10-11T00:01:00Z"},
+    "mesh/repo-check": {"state": "success", "description": "pass: its merge-check.sh passed", "updated_at": "2026-10-11T00:02:00Z"}
+  }
+}]}`)
+	w.h.SeeOpenPulls()
+	w.later(3 * Bounds[Proposed].For)
+	w.h.SeeOpenPulls()
+	if lines := w.h.Stalled(); len(lines) != 0 {
+		t.Fatalf("a head whose required checks answered is said: %+v", lines)
+	}
+}
+
+// A forge module that says only which contexts a head has, not their states — the gitea module before issue 438 —
+// is read as before: a head with any merge check is not said, and nothing is guessed of what it does not say.
+func TestAForgeThatSaysNoStatesIsReadAsBefore(t *testing.T) {
+	w := newWorld(t)
+	w.forge.open = openPullsOf(t, `{"pulls": [{"base": "main", "head_sha": "c11222026a3b26a0cc4b277119e1ea059b82a36b",
+  "number": 212, "owner": "novox", "repo": "mesh-controller", "required": ["mesh/merge-gate", "mesh/repo-check"],
+  "statuses": ["mesh/merge-gate", "mesh/delivery"]}]}`)
+	w.h.SeeOpenPulls()
+	w.later(3 * Bounds[Proposed].For)
+	w.h.SeeOpenPulls()
+	if lines := w.h.Stalled(); len(lines) != 0 {
+		t.Fatalf("a head is said on states the forge did not give: %+v", lines)
+	}
+}
@@ -3,6 +3,7 @@ package main
 import (
 	"fmt"
 	"slices"
+	"strings"
 	"time"
 )

@@ -15,6 +16,15 @@ import (
 // since this owner first saw it so is said as a delivery held: a `stalled` line in the state Unannounced, which the
 // controller raises as `delivery.<id>.stalled` (D14) like any other delivery past its bound. The delivery domain's own
 // word, so no second condition and no second judge: the id is the one its delivery will have once announced.
+//
+// A head whose check started and never answered is said the same way (novox/hq issue 438). Counting any merge-check
+// status as checked let a head that got only a pending mesh/merge-gate, or never got mesh/repo-check, wait unable to
+// merge with nothing saying so. So a head is checked only when every merge check its base requires has answered —
+// success, failure, error or warning, a failure being a verdict the pull request shows itself — and one with a
+// required check pending or never set is said once that has lasted the delivery table's bound for a check asked and
+// not answered (Bounds[Proposed]): an announced head would be said after that bound too, and it is longer than the
+// controller lets one check run (its checkTimeout, 45 minutes), so a slow check or a status read a moment behind
+// (issue 423) is never said.

 // Unannounced is a pull request's head the forge never announced: not a state of the table — no delivery exists —
 // only what a stalled line says.
@@ -29,7 +39,8 @@ const UncheckedAfter = 10 * time.Minute
 var mergeChecks = []string{"mesh/merge-gate", "mesh/repo-check"}

 // OpenPull is an open pull request as the forge says it (the gitea module's gitea_open_pulls): its head, its base,
-// the contexts the base's protection requires, and the contexts its head has a status of, in any state.
+// the contexts the base's protection requires, the contexts its head has a status of, in any state, and each of
+// those statuses with its state.
 type OpenPull struct {
 	Owner    string   `json:"owner"`
 	Repo     string   `json:"repo"`
@@ -39,24 +50,64 @@ type OpenPull struct {
 	HTMLURL  string   `json:"html_url,omitempty"`
 	Required []string `json:"required"`
 	Statuses []string `json:"statuses"`
+	// Checks are the head's statuses by context, the newest of each (novox/hq issue 438). A forge module older
+	// than that says none: the head is then read by Statuses alone, as before, and a pending check is not told
+	// from an answered one.
+	Checks map[string]HeadCheck `json:"checks,omitempty"`
 }

-// unchecked is a head seen without a merge check, and when this owner first saw it so — its own time, since the
-// forge's update time moves on every comment.
+// HeadCheck is one status on a head as the forge keeps it: pending, success, failure, error or warning.
+type HeadCheck struct {
+	State       string `json:"state"`
+	Description string `json:"description,omitempty"`
+	UpdatedAt   string `json:"updated_at,omitempty"`
+}
+
+// unchecked is a head seen without an answered merge check, what it lacks, and when this owner first saw it so —
+// its own time, since the forge's update time moves on every comment.
 type unchecked struct {
-	pull  OpenPull
-	first time.Time
+	pull    OpenPull
+	waiting []string
+	first   time.Time
 }

-// wantsACheck is whether the base requires the mesh's merge check and the head has none of it.
-func (p OpenPull) wantsACheck() bool {
-	required := slices.ContainsFunc(mergeChecks, func(c string) bool { return slices.Contains(p.Required, c) })
-	checked := slices.ContainsFunc(mergeChecks, func(c string) bool { return slices.Contains(p.Statuses, c) })
-	return required && !checked && p.HeadSHA != ""
+// unanswered are the merge checks the base requires that the head has no verdict of, each as it reads — "never
+// set", or pending and since when — and whether the head has any merge check at all: its check started. A head on
+// a base that requires none, or without a head, has nothing unanswered.
+func (p OpenPull) unanswered() (waiting []string, started bool) {
+	if p.HeadSHA == "" {
+		return nil, false
+	}
+	started = slices.ContainsFunc(mergeChecks, func(c string) bool {
+		return slices.Contains(p.Statuses, c) || p.Checks[c].State != ""
+	})
+	for _, c := range mergeChecks {
+		if !slices.Contains(p.Required, c) {
+			continue
+		}
+		got, ok := p.Checks[c]
+		switch {
+		case p.Checks == nil && started:
+			// A forge that says no states: a head with any merge check counts as checked, as before issue 438,
+			// since nothing here says whether the one it has answered.
+		case !ok || got.State == "":
+			waiting = append(waiting, c+" never set")
+		case got.State == "pending":
+			waiting = append(waiting, c+" pending since "+orUnknown(got.UpdatedAt))
+		}
+	}
+	return waiting, started
 }

-// SeeOpenPulls asks the forge for every open pull request and keeps the heads that want a check and have none.
-// A forge that cannot be asked changes nothing: what was seen stands, and nothing is guessed.
+func orUnknown(at string) string {
+	if at == "" {
+		return "a time the forge did not say"
+	}
+	return at
+}
+
+// SeeOpenPulls asks the forge for every open pull request and keeps the heads with a required merge check not
+// answered. A forge that cannot be asked changes nothing: what was seen stands, and nothing is guessed.
 func (h *Holder) SeeOpenPulls() {
 	open, err := h.Forge.OpenPulls()
 	if err != nil {
@@ -68,38 +119,73 @@ func (h *Holder) SeeOpenPulls() {
 	now := h.Now()
 	seen := map[string]unchecked{}
 	for _, p := range open {
-		if !p.wantsACheck() {
+		waiting, _ := p.unanswered()
+		if len(waiting) == 0 {
 			continue
 		}
 		id := IDOf(p.Owner+"/"+p.Repo, p.HeadSHA)
 		if h.deliveries[id] != nil {
 			continue // announced: a delivery with its own bound
 		}
+		// The clock runs from when this owner first saw the head waiting as it waits now: a check that started, or
+		// was asked again — its pending status set anew — is given its whole bound from then.
 		first := now
-		if was, ok := h.unchecked[id]; ok {
+		if was, ok := h.unchecked[id]; ok && slices.Equal(was.waiting, waiting) {
 			first = was.first
 		} else {
-			h.Logf("[mesh-delivery] %s/%s#%d's head %s has no merge check; said if it has none in %s", p.Owner, p.Repo,
-				p.Number, short(p.HeadSHA), UncheckedAfter)
+			u := unchecked{pull: p, waiting: waiting}
+			h.Logf("[mesh-delivery] %s/%s#%d's head %s %s; said if so in %s", p.Owner, p.Repo, p.Number,
+				short(p.HeadSHA), u.what(), u.bound())
 		}
-		seen[id] = unchecked{pull: p, first: first}
+		seen[id] = unchecked{pull: p, waiting: waiting, first: first}
 	}
 	h.unchecked = seen
 }

-// uncheckedLines are the heads unchecked past UncheckedAfter, as stalled lines; under the lock.
+// started is whether the head has any merge check: its check started, rather than was never asked.
+func (u unchecked) started() bool {
+	_, started := u.pull.unanswered()
+	return started
+}
+
+// bound is how long the head may wait as it does: UncheckedAfter for a check never asked, the table's bound for a
+// check asked and not answered for one that started (novox/hq issue 438).
+func (u unchecked) bound() time.Duration {
+	if u.started() {
+		return Bounds[Proposed].For
+	}
+	return UncheckedAfter
+}
+
+// what is what the head lacks, in words.
+func (u unchecked) what() string {
+	if u.started() {
+		return "has a merge check that has not answered: " + strings.Join(u.waiting, ", ")
+	}
+	return "has no merge check"
+}
+
+// uncheckedLines are the heads unchecked past their bound, as stalled lines; under the lock.
 func (h *Holder) uncheckedLines(now time.Time) []StalledLine {
 	var out []StalledLine
 	for id, u := range h.unchecked {
-		if h.deliveries[id] != nil || now.Sub(u.first) < UncheckedAfter {
+		if h.deliveries[id] != nil || now.Sub(u.first) < u.bound() {
 			continue
 		}
+		h2 := "none: the forge never announced it, so there is no delivery to close — the operator's"
+		says := fmt.Sprintf("%s/%s#%d is open on %s, which requires the merge check, and its head has had no merge "+
+			"check: the mesh was never asked to check it. A new commit on its branch announces it",
+			u.pull.Owner, u.pull.Repo, u.pull.Number, u.pull.Base)
+		if u.started() {
+			says = fmt.Sprintf("%s/%s#%d is open on %s, which requires the merge check, and its head's check started "+
+				"and never answered: %s. No delivery of it is held here, so nothing asks it again. A new commit on "+
+				"its branch announces it and asks its check", u.pull.Owner, u.pull.Repo, u.pull.Number, u.pull.Base,
+				strings.Join(u.waiting, ", "))
+			h2 = "none: no delivery of it is held here, so there is none to close — the operator's"
+		}
 		out = append(out, StalledLine{ID: id, Number: u.pull.Number, State: Unannounced, For: now.Sub(u.first).Round(time.Second).String(),
-			Bound: UncheckedAfter.String(),
-			H2:    "none: the forge never announced it, so there is no delivery to close — the operator's",
-			Says: fmt.Sprintf("%s/%s#%d is open on %s, which requires the merge check, and its head has had no merge "+
-				"check: the mesh was never asked to check it. A new commit on its branch announces it",
-				u.pull.Owner, u.pull.Repo, u.pull.Number, u.pull.Base)})
+			Bound: u.bound().String(),
+			H2:    h2, Says: says})
 	}
 	return out
 }