mesh-issues: end a fix-stuck warning when its issue is resolved, and never say a closed issue's fix stuck (issue 547)
mesh/merge-gate pass: builds mesh-issues → novox; no bus step; every machine composes with the change as it did without (4 of 4 compose)
mesh/repo-check pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-check-typescript.sh (typescript)
mesh/delivery delivered
mesh/merge-gate pass: builds mesh-issues → novox; no bus step; every machine composes with the change as it did without (4 of 4 compose)
mesh/repo-check pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-check-typescript.sh (typescript)
mesh/delivery delivered
A resolved issue's worker proved the acceptance live, so a fix whose own walk failed and that a later merge carried is not waited for. Only wontfix ended the warning, so 16 warnings stayed open on the operator's channel and the sweep raised new ones for resolved issues.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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 ||
|
||||
|
||||
@@ -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 := ¬ified{}
|
||||
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 := ¬ified{}
|
||||
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 := ¬ified{}
|
||||
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 := ¬ified{}
|
||||
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 := ¬ified{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)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user