Merge pull request 'mesh-issues: a resolved issue ends its fix-stuck warning, and a closed issue's fix is never said stuck (hq issue 547)' (#239) from fix/547-resolved-ends-stuck into main

modules/mesh-issues/README.md

@@ -134,9 +134,12 @@ carries `abandoned` on `get` and its `list` row, beside its `worker`. A move by
 `take-over`'s reason, as any stale worker; the answer, the note and the `moved` event say it was abandoned
 (`abandoned`).
 
-Every five minutes the module looks for a fix merged and not delivered after `stuck-after` (default `2h`): it is
-said once — a note on the issue and a warning on the operator's channel (`seat:operator-channel.notify`, key
-`issue-tracker.fix-stuck.<number>`), cleared there when every fix of the issue is delivered. A module that runs only
+Every five minutes the module looks for a fix merged and not delivered after `stuck-after` (default `2h`) of an
+issue not closed: it is said once — a note on the issue and a warning on the operator's channel
+(`seat:operator-channel.notify`, key `issue-tracker.fix-stuck.<number>`), cleared there when every fix of the issue
+is delivered or the issue is closed. Resolved is the end of the warning as wontfix is: its worker proved the
+acceptance live, so a fix whose own walk failed and that a later merge carried is not waited for (hq issue 547). An
+issue resolved or wontfix is never said stuck. A module that runs only
 its bundle cannot raise a controller condition (hq ADR 0240 rule 8, ADR 0256), so the warning is the channel's, not the controller's.
 
 ## The import of hq's issues (slice 3)

modules/mesh-issues/cmd/mesh-issues/delivery.go

@@ -266,9 +266,11 @@ func (t *Tracker) afterFix(n uint64) error {
 }
 
 // stillStuck says whether an issue has a fix said stuck that is still named, not delivered, and the issue not
-// closed as wontfix.
+// closed. A closed issue is the end of its warning: resolved means its worker proved the acceptance live, so a fix
+// the tracker never heard delivered is not waited for any more (a failed walk that a later merge carried stays
+// failed, novox/hq issue 547), and wontfix waits for nothing.
 func stillStuck(is *Issue) bool {
-	if is.Status == StatusWontfix {
+	if closedStatus(is.Status) {
 		return false
 	}
 	named := liveFixes(is)
@@ -281,19 +283,30 @@ func stillStuck(is *Issue) bool {
 }
 
 // clearStuck clears the warning said for an issue on the operator's channel once nothing it was said for is
-// stuck — delivered, no longer named, or the issue closed as wontfix; one that fails is tried again at the next
-// sweep.
+// stuck — delivered, no longer named, or the issue closed (resolved or wontfix); one that fails is tried again at
+// the next sweep.
 func (t *Tracker) clearStuck(is *Issue) error {
 	if !is.StuckOpen || stillStuck(is) || t.Notify == nil {
 		return nil
 	}
-	if err := t.Notify(stuckKey(is.Number), "", "", "", "", "the fix of issue "+keyOf(is.Number)+" is delivered or no longer waited for", true); err != nil {
+	if err := t.Notify(stuckKey(is.Number), "", "", "", "", stuckEnded(is), true); err != nil {
 		return err
 	}
 	_, err := t.change(is.Number, Act{Op: "warned", By: Seat}, func(i *Issue) error { i.StuckOpen = false; return nil })
 	return err
 }
 
+// stuckEnded is what the operator's channel says when a stuck warning ends.
+func stuckEnded(is *Issue) string {
+	switch is.Status {
+	case StatusResolved:
+		return "resolved: issue " + keyOf(is.Number) + " was proven live by its worker"
+	case StatusWontfix:
+		return "issue " + keyOf(is.Number) + " was closed as wontfix: its fix is no longer waited for"
+	}
+	return "the fix of issue " + keyOf(is.Number) + " is delivered or no longer waited for"
+}
+
 func stuckKey(n uint64) string { return "issue-tracker.fix-stuck." + keyOf(n) }
 
 func prKey(repository string, number int) string {
@@ -453,7 +466,7 @@ func (t *Tracker) stuckAfter() time.Duration {
 	return DefaultStuckAfter
 }
 
-// Sweep says every fix merged and not delivered past the bound: a note on its issue and a warning on the
+// Sweep says every fix of an issue not closed merged and not delivered past the bound: a note on its issue and a warning on the
 // operator's channel, once; and every issue in fixing whose fixes were all delivered past its own bound and is not
 // resolved (novox/hq issue 483). First it asks the delivery of a few fixes never heard of. It answers how many
 // warnings it said. Run every few minutes.
@@ -474,15 +487,13 @@ func (t *Tracker) Sweep() (int, error) {
 		} else if ok {
 			said++
 		}
-		if is.Status == StatusWontfix {
-			if err := t.clearStuck(is); err != nil {
-				t.Logf("[mesh-issues] the stuck fix of %d could not be cleared on the operator's channel: %v", n, err)
-			}
-			continue
-		}
 		if err := t.clearStuck(is); err != nil {
 			t.Logf("[mesh-issues] the stuck fix of %d could not be cleared on the operator's channel: %v", n, err)
 		}
+		if closedStatus(is.Status) {
+			// A resolved or wontfix issue waits for no fix: never said stuck (novox/hq issue 547).
+			continue
+		}
 		named := liveFixes(is)
 		for _, f := range is.Fixes {
 			if !contains(named, f.PullRequest) || contains(is.Delivered, f.PullRequest) || !f.Merged || f.MergedAt == nil ||

modules/mesh-issues/cmd/mesh-issues/delivery_test.go

@@ -11,10 +11,22 @@ import (
 )
 
 type notified struct {
-	mu    sync.Mutex
-	said  []string
-	clear []string
-	err   error
+	mu       sync.Mutex
+	said     []string
+	clear    []string
+	resolved []string
+	err      error
+}
+
+// fixedAtFixing names a fix while the issue stays at fixing, as an agent does when the fix merges (ADR 0281 §3,
+// ADR 0284): a fix stuck is said only for an issue not closed (issue 547).
+func fixedAtFixing(t *testing.T, h *harness, n uint64, fixedBy ...string) {
+	t.Helper()
+	h.fixing(t, n)
+	if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix@g14", LocatedIn: []string{"mesh-catalog modules/records"},
+		FixedBy: fixedBy}); err != nil {
+		t.Fatal(err)
+	}
 }
 
 func (n *notified) notify(key, severity, summary, headline, explanation, resolved string, clear bool) error {
@@ -25,6 +37,7 @@ func (n *notified) notify(key, severity, summary, headline, explanation, resolve
 	}
 	if clear {
 		n.clear = append(n.clear, key)
+		n.resolved = append(n.resolved, resolved)
 	} else {
 		n.said = append(n.said, key+" "+severity+" "+summary)
 	}
@@ -121,7 +134,7 @@ func TestAnIssueKnowsWhetherItsFixIsDelivered(t *testing.T) {
 func TestAStuckFixIsSaidOnceAndClearedWhenDelivered(t *testing.T) {
 	h := newHarness(t)
 	n := h.open(t, "the counter skips a number")
-	resolve(t, h, n, "mesh-catalog#9")
+	fixedAtFixing(t, h, n, "mesh-catalog#9")
 	note := &notified{}
 	h.Notify = note.notify
 	merged := time.Date(2026, 10, 10, 9, 0, 0, 0, time.UTC) // three hours before the harness's clock
@@ -152,7 +165,7 @@ func TestAStuckFixIsSaidOnceAndClearedWhenDelivered(t *testing.T) {
 	// Under the bound nothing is said.
 	h2 := newHarness(t)
 	m := h2.open(t, "y")
-	resolve(t, h2, m, "mesh-catalog#9")
+	fixedAtFixing(t, h2, m, "mesh-catalog#9")
 	n2 := &notified{}
 	h2.Notify = n2.notify
 	transition(t, h2, 9, "delivering", "", "abc", time.Date(2026, 10, 10, 11, 30, 0, 0, time.UTC))
@@ -194,7 +207,8 @@ func TestAFixIsNamedWhileTheIssueStaysLocated(t *testing.T) {
 func TestDeliveredStaysDeliveredAndClearsAreRetried(t *testing.T) {
 	h := newHarness(t)
 	n := h.open(t, "x")
-	resolve(t, h, n, "mesh-catalog#9")
+	fixedAtFixing(t, h, n, "mesh-catalog#9")
+	h.settings.DeliveredNotResolvedAfter = "24h" // only the stuck warning is under test here
 	note := &notified{}
 	h.Notify = note.notify
 	merged := time.Date(2026, 10, 10, 9, 0, 0, 0, time.UTC)
@@ -244,3 +258,78 @@ func TestDeliveredStaysDeliveredAndClearsAreRetried(t *testing.T) {
 		t.Fatalf("%d asks, %q", asks, v.Delivery)
 	}
 }
+
+// A resolved issue is the end of its stuck warning, as wontfix is: resolving clears it on the operator's channel,
+// saying so, and the sweep never says a fix stuck for an issue resolved or wontfix, even one whose delivery failed
+// and never reads delivered (issue 547).
+func TestAResolvedIssueEndsItsStuckWarningAndIsNeverSaidStuck(t *testing.T) {
+	merged := time.Date(2026, 10, 10, 9, 0, 0, 0, time.UTC) // three hours before the harness's clock
+	moved := func(t *testing.T, h *harness, n uint64) {
+		t.Helper()
+		raw, _ := json.Marshal(map[string]any{"number": n})
+		if err := h.OnMoved(stdio.Envelope{Body: raw}); err != nil {
+			t.Fatal(err)
+		}
+	}
+	for _, to := range []string{StatusResolved, StatusWontfix} {
+		t.Run(to, func(t *testing.T) {
+			h := newHarness(t)
+			n := h.open(t, "the counter skips a number")
+			fixedAtFixing(t, h, n, "mesh-catalog#9")
+			note := &notified{}
+			h.Notify = note.notify
+			transition(t, h, 9, "failed", "the gate failed on one machine", "abc", merged)
+			if said, err := h.Sweep(); err != nil || said != 1 || len(note.said) != 1 {
+				t.Fatalf("not said stuck: %d %v %q", said, err, note.said)
+			}
+			ask := MoveAsk{Number: n, Status: to, FixedBy: []string{"mesh-catalog#9"}, ReplayNone: "proven live"}
+			if to == StatusWontfix {
+				ask = MoveAsk{Number: n, Status: to, Reason: "not worth it"}
+			}
+			if _, err := h.Move(ask); err != nil {
+				t.Fatal(err)
+			}
+			moved(t, h, n)
+			if len(note.clear) != 1 || note.clear[0] != stuckKey(n) || get(t, h, n).StuckOpen {
+				t.Fatalf("not cleared at the move: %q", note.clear)
+			}
+			if to == StatusResolved && !strings.HasPrefix(note.resolved[0], "resolved: ") {
+				t.Fatalf("%q", note.resolved)
+			}
+			if said, _ := h.Sweep(); said != 0 || len(note.said) != 1 || len(note.clear) != 1 {
+				t.Fatalf("said again after %s: %d %q, cleared %q", to, said, note.said, note.clear)
+			}
+		})
+	}
+	// A warning left open by a tracker that kept it for a resolved issue is cleared at the first sweep, and an
+	// issue resolved before its fix was ever said stuck is never said stuck.
+	h := newHarness(t)
+	n := h.open(t, "x")
+	fixedAtFixing(t, h, n, "mesh-catalog#9")
+	note := &notified{err: errStuck}
+	h.Notify = note.notify
+	transition(t, h, 9, "failed", "the gate failed", "abc", merged)
+	if _, err := h.change(n, Act{Op: "warned", By: Seat}, func(i *Issue) error {
+		i.Fixes[0].StuckSaid, i.StuckOpen = true, true
+		return nil
+	}); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := h.Move(MoveAsk{Number: n, Status: StatusResolved, FixedBy: []string{"mesh-catalog#9"}}); err != nil {
+		t.Fatal(err)
+	}
+	moved(t, h, n) // the channel does not answer: kept open, tried at the sweep
+	note.err = nil
+	if said, _ := h.Sweep(); said != 0 || len(note.clear) != 1 || get(t, h, n).StuckOpen {
+		t.Fatalf("said %d, cleared %q", said, note.clear)
+	}
+	m := h.open(t, "y")
+	fixedAtFixing(t, h, m, "mesh-catalog#10")
+	transition(t, h, 10, "failed", "the gate failed", "abc", merged)
+	if _, err := h.Move(MoveAsk{Number: m, Status: StatusResolved, FixedBy: []string{"mesh-catalog#10"}}); err != nil {
+		t.Fatal(err)
+	}
+	if said, _ := h.Sweep(); said != 0 || len(note.said) != 0 {
+		t.Fatalf("a resolved issue said stuck: %q", note.said)
+	}
+}