From ee1d7e5cae12153ab82253d5fdec7f8ec407d5ea Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 13 Oct 2026 00:31:57 +0200 Subject: [PATCH] Mark a rerun check with a short (run N) after its verdict, and word a re-hand-out neutrally The forge clips a status to 140 characters, so the long sentence pushed the verdict's own words out; and a paused holder's hand-back or a bus delay also count as a delivery, so the cause is not claimed (novox/hq issue 594). --- cmd/mesh-controller/checks.go | 13 +++++++------ cmd/mesh-controller/queue.go | 2 +- cmd/mesh-controller/restarted_test.go | 14 +++++++++----- internal/link/builds.go | 19 +++++++++++++++---- 4 files changed, 32 insertions(+), 16 deletions(-) diff --git a/cmd/mesh-controller/checks.go b/cmd/mesh-controller/checks.go index a9b21ee2..8a8265eb 100644 --- a/cmd/mesh-controller/checks.go +++ b/cmd/mesh-controller/checks.go @@ -429,18 +429,19 @@ func checkedOf(result link.BuildResult) link.Checked { l.Verdict = "error" } } - // **A check run again says so on every layer** (novox/hq issue 594): the summaries are the lines the - // pull request shows, and a verdict from a second run — an earlier build agent replaced mid-check — - // must not read as the first. Copies, so the build's own outcome is left as it came. - if said := link.Restarted(result.Delivery); said != "" { - c.Summary = said + "; " + c.Summary + // **A check run again says so** (novox/hq issue 594). The summaries are the lines the pull request + // shows, clipped by the forge to a few words, so they get only a short marker after the verdict's own + // words; the report gets the whole sentence. Layers copied, so the build's own outcome is left as it came. + if mark := link.RunMarker(result.Delivery); mark != "" { + c.Summary += " " + mark for _, l := range []**link.CheckLayer{&c.Gate, &c.RepoCheck} { if *l != nil { layer := **l - layer.Summary = said + "; " + layer.Summary + layer.Summary += " " + mark *l = &layer } } + c.Report = link.Restarted(result.Delivery) + "\n\n" + c.Report } if len(c.Report) > maxCheckReport { c.Report = "…" + c.Report[len(c.Report)-maxCheckReport:] diff --git a/cmd/mesh-controller/queue.go b/cmd/mesh-controller/queue.go index 08439d52..629fe5d4 100644 --- a/cmd/mesh-controller/queue.go +++ b/cmd/mesh-controller/queue.go @@ -212,7 +212,7 @@ func queueText(q link.Queue, now time.Time) string { where = fmt.Sprintf("on %s for %s", a.On, now.Sub(a.Started).Round(time.Second)) // Started again after an earlier taker left it (novox/hq issue 594). if a.Delivery > 1 { - where += fmt.Sprintf(", restarted (handed out %d times)", a.Delivery) + where += fmt.Sprintf(", handed out %d times", a.Delivery) } case a.Was != "": where = fmt.Sprintf("handed back by %s, to be handed out again", a.Was) diff --git a/cmd/mesh-controller/restarted_test.go b/cmd/mesh-controller/restarted_test.go index 6e169970..380ffa37 100644 --- a/cmd/mesh-controller/restarted_test.go +++ b/cmd/mesh-controller/restarted_test.go @@ -19,10 +19,14 @@ func TestARestartedCheckSaysSoOnEveryLayer(t *testing.T) { Check: &link.CheckOutcome{Verdict: "pass", Summary: "every node composes", Gate: gate, RepoCheck: repo}}) for name, said := range map[string]string{"summary": c.Summary, "gate": c.Gate.Summary, "repository check": c.RepoCheck.Summary} { - if !strings.Contains(said, "restarted") { - t.Errorf("the %s says %q: a check run twice reads as run once", name, said) + // The verdict's own words first, then the marker: the forge clips a status to a few words. + if !strings.HasSuffix(said, " (run 2)") || strings.HasPrefix(said, "(run") { + t.Errorf("the %s says %q: a check run twice reads as run once, or its marker pushes its words out", name, said) } } + if !strings.HasPrefix(c.Report, "restarted: handed out 2 times") { + t.Errorf("the report does not open with the restart: %q", c.Report) + } if gate.Summary != "every node composes" { t.Errorf("the build's own outcome was changed under it: %q", gate.Summary) } @@ -30,7 +34,7 @@ func TestARestartedCheckSaysSoOnEveryLayer(t *testing.T) { // Taken once, nothing is added. once := checkedOf(link.BuildResult{ID: "b", Ref: "abc", Checked: asked, Delivery: 1, Check: &link.CheckOutcome{Verdict: "pass", Summary: "every node composes"}}) - if strings.Contains(once.Summary, "restarted") { + if strings.Contains(once.Summary, "(run") || strings.Contains(once.Report, "restarted") { t.Errorf("a check run once says %q", once.Summary) } } @@ -43,8 +47,8 @@ func TestTheQueueSaysAnAskInFlightRestarted(t *testing.T) { State: link.AskInFlight, On: "shanks", Started: now.Add(-time.Minute), Delivery: 2}, }} text := queueText(q, now) - if !strings.Contains(text, "restarted") { - t.Fatalf("the queue does not say the ask restarted:\n%s", text) + if !strings.Contains(text, "handed out 2 times") { + t.Fatalf("the queue does not say the ask was handed out again:\n%s", text) } if a := queueAnswerOf(q, now).Asks[0]; a.Delivery != 2 { t.Errorf("the queue's data says delivery %d, want 2", a.Delivery) diff --git a/internal/link/builds.go b/internal/link/builds.go index 43b52272..7fe20541 100644 --- a/internal/link/builds.go +++ b/internal/link/builds.go @@ -98,14 +98,25 @@ type BuildStart struct { Delivery int `json:"delivery,omitempty"` } -// Restarted is how a build handed out again says so (novox/hq issue 594): in its log, in the queue and on -// the pull request its check answers, so a second run never reads as the first. Empty for a first start. +// Restarted is how a build handed out again says so (novox/hq issue 594), in its log and in its check's +// report, so a second run never reads as the first. Neutral on why: a holder replaced mid-build, a paused +// holder handing an ask back as it arrived, and a bus that held its acknowledgements past the wait all +// count as a hand-out. Empty for a first start. func Restarted(delivery int) string { if delivery < 2 { return "" } - return fmt.Sprintf("restarted: handed out %d times — an earlier build agent took it and did not finish, "+ - "most often because it was replaced mid-build, so it ran again from the start", delivery) + return fmt.Sprintf("restarted: handed out %d times; an earlier hand-out did not finish here, so this run "+ + "started from the beginning", delivery) +} + +// RunMarker is the short form of Restarted, for a line clipped to a few words — a check's status on the +// forge (novox/hq issue 594): "(run 2)". Empty for a first start. +func RunMarker(delivery int) string { + if delivery < 2 { + return "" + } + return fmt.Sprintf("(run %d)", delivery) } // BuildLine is one thing a build said while building.