Merge pull request 'End a stranded stop whose modules no merge can carry, so its alert clears instead of asking the operator for ever (issue 1581)' (#405) from fix/1581-stranded-removed into main

This commit was merged in pull request #405.
This commit is contained in:
2026-10-21 08:40:20 +00:00
4 changed files with 119 additions and 1 deletions
@@ -1430,7 +1430,7 @@ func (h *Holder) plan() []func() {
h.tellCovered(d, now) // covered before this holder told it, or before a restart (novox/hq issue 480)
continue
}
if strandedUncovered(d) {
if strandedOpen(d) {
// Stopped and final, but still read for a later merge that delivered its modules: until one did,
// `stalled` keeps listing it (review of novox/hq issue 474).
var ask []string
@@ -1606,6 +1606,12 @@ func (h *Holder) tellCovered(d *Delivery, now time.Time) {
// merged change that never reached the machines, which `stalled` keeps listing until a later delivered merge
// carries its modules (review of novox/hq issue 474).
func strandedUncovered(d *Delivery) bool {
return strandedOpen(d) && !d.strandedEnded()
}
// strandedOpen is a stop for a walk that built none of its modules that no merge was found to cover: still read for
// a cover, and listed unless it is ended (novox/hq issue 1581).
func strandedOpen(d *Delivery) bool {
return d.State == Stopped && d.Stranded != "" && d.CoveredBy == ""
}
@@ -0,0 +1,72 @@
package main
import (
"strings"
"testing"
"time"
)
// novox/hq issue 1581: a delivery stopped because its walk ended without building its modules (issue 474) waited for
// "a later delivered merge to carry" them, listed by `stalled` for good. When no later merge can: the pull request
// removed the module from the catalogue (mesh-catalog #384, scan), or the module is built for no machine (#391, "sent
// nowhere"), the stop reads ended, as covered reads covered (issue 480) — a reading of the stop, no transition — says
// why, and leaves `stalled`, so the condition that listed it ends by itself.
func strandedWith(t *testing.T, change func(d *Delivery)) (*world, string) {
w, id := strandedNight(t)
if len(w.h.Stalled()) != 1 {
t.Fatalf("before, stalled lists %+v", w.h.Stalled())
}
w.h.mu.Lock()
d := w.h.deliveries[id]
change(d)
if err := w.store.PutDelivery(d); err != nil {
t.Fatal(err)
}
w.h.mu.Unlock()
w.later(time.Minute)
w.settleAll()
return w, id
}
func TestAStrandedStopWhoseModuleIsSentNowhereEndsWithoutALaterMerge(t *testing.T) {
w, id := strandedWith(t, func(d *Delivery) {
d.Plan.Summary = "builds build-agent, sent nowhere; no bus step"
})
if s := w.h.Stalled(); len(s) != 0 {
t.Fatalf("sent nowhere, stalled still lists %+v", s)
}
shown, err := w.h.Show(id)
if err != nil {
t.Fatal(err)
}
reads, _ := shown.(map[string]any)["reads"].(string)
if !strings.HasPrefix(reads, "ended") || !strings.Contains(reads, "build-agent") {
t.Fatalf("show reads %q", reads)
}
if waits, _ := shown.(map[string]any)["waits"].(string); strings.Contains(waits, "later delivered merge") {
t.Fatalf("it still waits for %q", waits)
}
if d := w.delivery(id); d.State != Stopped {
t.Fatalf("a reading, not a transition: it is %s", d.State)
}
}
func TestAStrandedStopWhoseModuleThePullRequestRemovedEndsWithoutALaterMerge(t *testing.T) {
w, _ := strandedWith(t, func(d *Delivery) {
d.Removed = []string{"modules/build-agent/module.json", "modules/build-agent/cmd/main.go"}
})
if s := w.h.Stalled(); len(s) != 0 {
t.Fatalf("removed, stalled still lists %+v", s)
}
}
// A stop whose module still runs somewhere keeps waiting for a later merge, and stays listed.
func TestAStrandedStopWhoseModuleCanStillBeCarriedStaysListed(t *testing.T) {
w, _ := strandedWith(t, func(d *Delivery) {
d.Removed = []string{"modules/other/module.json"}
})
if s := w.h.Stalled(); len(s) != 1 {
t.Fatalf("stalled lists %+v", s)
}
}
@@ -357,6 +357,44 @@ func (d *Delivery) coveredStop() bool {
return false
}
// strandedEnded is whether a stop for a walk that built none of its modules (novox/hq issue 474), found covered by no
// merge, waits for nothing that can come: every module its walk left unbuilt is one no later merge can carry, because
// the pull request removed it from the catalogue (its module.json is among the removed paths) or its plan sends it to
// no machine ("sent nowhere"). It is a reading of the stop, as covered is (issue 480): the state stays stopped, and
// `stalled` no longer lists it, so the condition that listed it ends by itself (issue 1581).
func (d *Delivery) strandedEnded() bool {
if d.State != Stopped || d.Stranded == "" || d.CoveredBy != "" {
return false
}
missing := d.notMovedBy()
for _, m := range missing {
if !d.cannotBeCarried(m) {
return false
}
}
return len(missing) > 0
}
// cannotBeCarried is whether no later merge can deliver a module this delivery's plan moved: it was removed, or the
// plan sends it nowhere.
func (d *Delivery) cannotBeCarried(module string) bool {
if d.Plan == nil {
return false
}
for _, p := range d.Removed {
if strings.HasSuffix(p, "/"+module+"/module.json") {
return true
}
}
return strings.Contains(d.Plan.Summary, ", sent nowhere") && len(d.Plan.receiving(module)) == 0
}
// strandedEndedSaid is how an ended stop reads, in `show`.
func (d *Delivery) strandedEndedSaid() string {
return "ended: it waited for a later merge to carry " + strings.Join(d.notMovedBy(), ", ") +
", and none can: the module was removed from the catalogue or is built for no machine (novox/hq issue 1581)"
}
// coveredStopSaid is how a covered stop or failure reads: on the forge's status, in its view and note, and in `show`.
func (d *Delivery) coveredStopSaid() string {
ended := "its stop"
@@ -127,6 +127,8 @@ func (h *Holder) Show(id string) (any, error) {
out := map[string]any{"delivery": d, "waits": h.waitsFor(d)}
if d.coveredStop() {
out["reads"] = d.coveredStopSaid() // stopped, and its change reached the machines (novox/hq issue 480)
} else if d.strandedEnded() {
out["reads"] = d.strandedEndedSaid() // stopped, and nothing can carry what it waited for (issue 1581)
}
if g := h.groups[d.Group]; g != nil {
state, why := GroupState(g, h.membersOf(g))