From c4ea58aeb0fddc89c15ca2c7e79a511471b9201a Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 12:14:42 +0200 Subject: [PATCH 1/5] mesh-issues: test that a delivered fix and a stale claim are said (hq issue 483) Each test fails on main: a linked pull request is never measured, nothing warns of a delivered fix left in fixing, and list has no flag to find either. --- .../cmd/mesh-issues/claims_test.go | 307 ++++++++++++++++++ 1 file changed, 307 insertions(+) create mode 100644 modules/mesh-issues/cmd/mesh-issues/claims_test.go diff --git a/modules/mesh-issues/cmd/mesh-issues/claims_test.go b/modules/mesh-issues/cmd/mesh-issues/claims_test.go new file mode 100644 index 00000000..aee2f4e6 --- /dev/null +++ b/modules/mesh-issues/cmd/mesh-issues/claims_test.go @@ -0,0 +1,307 @@ +package main + +// Tests of novox/hq issue 483: an issue whose fix is delivered stays in fixing, and a claim nobody works on is +// never noticed. The issues the operator found (231, 402, 457, …) named their fix the way the live tracker keeps +// it: a link of kind pull-request, by the worker, while the issue stayed in fixing, with fixed-by given only at +// the resolve (issue-tracker.get 457, 2026-10-11). + +import ( + "encoding/json" + "strings" + "testing" + "time" + + stdio "git.novox.be/novox/mesh-sdk/go" +) + +// asJSON is a verb's answer as the tool runner hands it on: what a caller reads, whatever the Go types are. +func asJSON(t *testing.T, v any) map[string]any { + t.Helper() + raw, err := json.Marshal(v) + if err != nil { + t.Fatal(err) + } + var m map[string]any + if err := json.Unmarshal(raw, &m); err != nil { + t.Fatal(err) + } + return m +} + +// linkPR links a pull request the way the live issues do: kind pull-request, target #. +func linkPR(t *testing.T, h *harness, n uint64, target string) { + t.Helper() + if _, err := h.call(t, "link", map[string]any{"number": n, "kind": "pull-request", "target": target}); err != nil { + t.Fatal(err) + } +} + +func rowsOf(t *testing.T, h *harness, args map[string]any) []map[string]any { + t.Helper() + v, err := h.call(t, "list", args) + if err != nil { + t.Fatal(err) + } + var out []map[string]any + for _, r := range asJSON(t, v)["rows"].([]any) { + out = append(out, r.(map[string]any)) + } + return out +} + +// Part 1: when every pull request linked to an issue in fixing is delivered, the issue gains one note naming +// them, when each was delivered, and its worker, and the event ready-to-verify names the worker. +func TestALinkedFixDeliveredIsNotedOnceNamingItsWorker(t *testing.T) { + h := newHarness(t) + now := clocked(h) + n := h.open(t, "a build fails during the registry's clean-up", "mesh-controller") + if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-457@shanks"}); err != nil { + t.Fatal(err) + } + linkPR(t, h, n, "mesh-catalog#225") + linkPR(t, h, n, "mesh-catalog#226") + active := get(t, h, n).Worker.Active + merged := now.Add(-20 * time.Minute) + transition(t, h, 225, "published", "", "abc", merged) + if v, _ := h.View(n); v.Delivery != "fix merged, not delivered: mesh-catalog#225" { + t.Fatalf("a linked pull request's delivery is not measured: %q", v.Delivery) + } + delivered := now.Add(-10 * time.Minute) + transition(t, h, 225, "delivered", "", "abc", delivered) + if countEvents(h, EventReadyToVerify) != 0 { + t.Fatal("said delivered while mesh-catalog#226 is not") + } + transition(t, h, 226, "delivered", "", "def", delivered.Add(time.Minute)) + transition(t, h, 226, "delivered", "", "def", delivered.Add(time.Minute)) + is := get(t, h, n) + var said []string + for _, nt := range is.Notes { + if strings.HasPrefix(nt.Text, "Ready to verify:") { + said = append(said, nt.Text) + } + } + if len(said) != 1 { + t.Fatalf("want one note, got %q", said) + } + for _, want := range []string{"mesh-catalog#225", "mesh-catalog#226", delivered.Format(time.RFC3339), + delivered.Add(time.Minute).Format(time.RFC3339), "fix-457@shanks"} { + if !strings.Contains(said[0], want) { + t.Fatalf("the note does not say %q: %s", want, said[0]) + } + } + if is.Status != StatusFixing || is.Worker == nil || is.Worker.By != "fix-457@shanks" { + t.Fatalf("the tracker's own note changed the issue's work: %+v", is) + } + var ev map[string]any + for _, e := range h.said() { + if e.Event == EventReadyToVerify { + ev = e.Body + } + } + if countEvents(h, EventReadyToVerify) != 1 || ev["worker"] != "fix-457@shanks" { + t.Fatalf("%d ready-to-verify, last %v", countEvents(h, EventReadyToVerify), ev) + } + // The tracker's own note keeps no worker current: the worker's activity is still its own last act. + if !is.Worker.Active.Equal(active) { + t.Fatalf("the worker was made active by the tracker's note: %v", is.Worker.Active) + } +} + +// Part 2: past the bound, an issue in fixing whose every linked fix is delivered is warned on the operator's +// channel, once, naming the issue and its worker; the warning clears when the issue leaves fixing. +func TestDeliveredNotResolvedIsWarnedAfterTheBoundAndClearedWhenItLeavesFixing(t *testing.T) { + h := newHarness(t) + now := clocked(h) + note := ¬ified{} + h.Notify = note.notify + n := h.open(t, "x", "mesh-catalog modules/records") + if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-x@g14"}); err != nil { + t.Fatal(err) + } + linkPR(t, h, n, "mesh-catalog#9") + transition(t, h, 9, "delivered", "", "abc", *now) + *now = now.Add(59 * time.Minute) + if _, err := h.Sweep(); err != nil || len(note.said) != 0 { + t.Fatalf("warned under the bound: %q %v", note.said, err) + } + if rows := rowsOf(t, h, map[string]any{"flag": "delivered-not-resolved"}); len(rows) != 1 || + rows[0]["delivered-not-resolved"] != true || rows[0]["worker"] != "fix-x@g14" { + t.Fatalf("list flag delivered-not-resolved: %v", rows) + } + *now = now.Add(2 * time.Minute) + for i := 0; i < 2; i++ { + if _, err := h.Sweep(); err != nil { + t.Fatal(err) + } + } + key := "issue-tracker.delivered-not-resolved.365" + if len(note.said) != 1 || !strings.HasPrefix(note.said[0], key+" warning ") || + !strings.Contains(note.said[0], "fixed and delivered, not resolved") || !strings.Contains(note.said[0], "fix-x@g14") || + !strings.Contains(note.said[0], "365") { + t.Fatalf("said %q", note.said) + } + if v := asJSON(t, mustView(t, h, n)); v["delivered-not-resolved"] != true { + t.Fatalf("get does not say it: %v", v["delivered-not-resolved"]) + } + if _, err := h.Move(MoveAsk{Number: n, Status: StatusResolved, FixedBy: []string{"mesh-catalog#9"}}); err != nil { + t.Fatal(err) + } + if err := h.OnMoved(envelopeOf(n)); err != nil { + t.Fatal(err) + } + if len(note.clear) != 1 || note.clear[0] != key { + t.Fatalf("not cleared when it left fixing: %q", note.clear) + } + if rows := rowsOf(t, h, map[string]any{"flag": "delivered-not-resolved"}); len(rows) != 0 { + t.Fatalf("a resolved issue still listed: %v", rows) + } + // The setting's bound is read. + h2 := newHarness(t) + now2 := clocked(h2) + n2 := ¬ified{} + h2.Notify = n2.notify + // Set as the operator's settings file gives it. + if err := json.Unmarshal([]byte(`{"delivered-not-resolved-after": "10m"}`), &h2.settings); err != nil { + t.Fatal(err) + } + m := h2.open(t, "y", "mesh-catalog modules/records") + if _, err := h2.Move(MoveAsk{Number: m, Status: StatusFixing, By: "fix-y@g14"}); err != nil { + t.Fatal(err) + } + linkPR(t, h2, m, "mesh-catalog#9") + transition(t, h2, 9, "delivered", "", "abc", *now2) + *now2 = now2.Add(11 * time.Minute) + if _, err := h2.Sweep(); err != nil || len(n2.said) != 1 { + t.Fatalf("the setting's bound was not read: %q %v", n2.said, err) + } +} + +// A warning the channel did not take is said at the next sweep, and an issue's own status is what clears it at +// the sweep too, when the moved event was not heard. +func TestDeliveredNotResolvedIsRetriedAndClearedAtTheSweep(t *testing.T) { + h := newHarness(t) + now := clocked(h) + note := ¬ified{err: errStuck} + h.Notify = note.notify + n := h.open(t, "x", "mesh-catalog modules/records") + if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-x@g14"}); err != nil { + t.Fatal(err) + } + linkPR(t, h, n, "mesh-catalog#9") + transition(t, h, 9, "delivered", "", "abc", *now) + *now = now.Add(2 * time.Hour) + _, _ = h.Sweep() + note.err = nil + _, _ = h.Sweep() + if len(note.said) != 1 { + t.Fatalf("not said at the next sweep: %q", note.said) + } + if _, err := h.Move(MoveAsk{Number: n, Status: StatusLocated}); err != nil { + t.Fatal(err) + } + _, _ = h.Sweep() + if len(note.clear) != 1 { + t.Fatalf("not cleared at the sweep: %q", note.clear) + } +} + +// Part 3: an issue in fixing with no open linked pull request and nothing done on it for a day is a stale claim, +// naming its worker; a new worker takes it over without a reason, and the tracker says the claim was stale. +func TestAStaleClaimIsFlaggedAndTakenOverWithoutAReason(t *testing.T) { + h := newHarness(t) + now := clocked(h) + n := h.open(t, "claimed and left", "hq") + if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "build-424@shanks"}); err != nil { + t.Fatal(err) + } + busy := h.open(t, "claimed, with a pull request in review", "hq") + if _, err := h.Move(MoveAsk{Number: busy, Status: StatusFixing, By: "fix-b@g14"}); err != nil { + t.Fatal(err) + } + linkPR(t, h, busy, "mesh-catalog#9") + transition(t, h, 9, "ready", "its merge", "", *now) + *now = now.Add(23 * time.Hour) + if rows := rowsOf(t, h, map[string]any{"flag": "stale-claim"}); len(rows) != 0 { + t.Fatalf("stale before the day: %v", rows) + } + *now = now.Add(2 * time.Hour) + rows := rowsOf(t, h, map[string]any{"flag": "stale-claim"}) + if len(rows) != 1 || rows[0]["number"] != float64(n) || rows[0]["worker"] != "build-424@shanks" || rows[0]["stale-claim"] != true { + t.Fatalf("want only %d flagged naming its worker, got %v", n, rows) + } + if v := asJSON(t, mustView(t, h, n)); v["stale-claim"] != true { + t.Fatalf("get does not say it: %v", v) + } + if v := asJSON(t, mustView(t, h, busy)); v["stale-claim"] == true { + t.Fatal("an issue with an open pull request is flagged") + } + m, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-483@shanks"}) + if err != nil { + t.Fatalf("a stale claim was not taken over without a reason: %v", err) + } + if a := asJSON(t, m); a["stale-claim"] != true || !strings.Contains(m.Said, "stale claim") { + t.Fatalf("the answer does not say the claim was stale: %v", a) + } + is := get(t, h, n) + last := is.Notes[len(is.Notes)-1].Text + if !strings.Contains(last, "stale claim") || !strings.Contains(last, "build-424@shanks") { + t.Fatalf("the note does not say the claim was stale: %s", last) + } + for _, e := range h.said() { + if e.Event == EventMoved && e.Body["took-over-from"] == "build-424@shanks" && e.Body["stale-claim"] != true { + t.Fatalf("the moved event does not say the claim was stale: %v", e.Body) + } + } +} + +// Part 4: list refuses a flag it does not know, naming the two it does. +func TestListRefusesAnUnknownFlag(t *testing.T) { + h := newHarness(t) + _, err := h.call(t, "list", map[string]any{"flag": "stuck"}) + if err == nil || !strings.Contains(err.Error(), "delivered-not-resolved") || !strings.Contains(err.Error(), "stale-claim") { + t.Fatalf("want refused naming the flags, got %v", err) + } +} + +// The sweep asks the delivery of a linked pull request it never heard of, a few per sweep, so an issue whose +// pull request merged before it was linked (as the operator's thirteen did) is measured at all. +func TestTheSweepAsksALinkedPullRequestNeverHeardOf(t *testing.T) { + h := newHarness(t) + now := clocked(h) + n := h.open(t, "x", "mesh-catalog modules/records") + if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-x@g14"}); err != nil { + t.Fatal(err) + } + linkPR(t, h, n, "mesh-catalog#209") + var asked []string + h.Deliveries = func(repo, number string) (Delivery, error) { + asked = append(asked, repo+"#"+number) + return Delivery{State: "delivered", Since: now.Add(-3 * time.Hour)}, nil + } + if _, err := h.Sweep(); err != nil { + t.Fatal(err) + } + if len(asked) != 1 || asked[0] != "mesh-catalog#209" { + t.Fatalf("asked %q", asked) + } + if v, _ := h.View(n); v.Delivery != "delivered to every machine it should reach" || countEvents(h, EventReadyToVerify) != 1 { + t.Fatalf("%q, %d ready-to-verify", v.Delivery, countEvents(h, EventReadyToVerify)) + } + if _, err := h.Sweep(); err != nil || len(asked) != 1 { + t.Fatalf("asked again once heard: %q %v", asked, err) + } +} + +func envelopeOf(n uint64) stdio.Envelope { + raw, _ := json.Marshal(map[string]any{"number": n}) + return stdio.Envelope{Key: "mesh-issues.moved", Body: raw} +} + +func mustView(t *testing.T, h *harness, n uint64) *View { + t.Helper() + v, err := h.View(n) + if err != nil { + t.Fatal(err) + } + return v +} From d81e7d063be152e98fc996be04622e1c32d848ae Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 12:16:59 +0200 Subject: [PATCH 2/5] mesh-issues: say a delivered fix left in fixing, and flag a stale claim (hq issue 483) The operator found 13 issues in fixing whose fix was delivered hours before: their pull requests were linked, not in fixed-by, so their delivery was never measured, and nothing said an issue left there or a worker gone. --- modules/mesh-issues/README.md | 27 ++- modules/mesh-issues/cmd/mesh-issues/claims.go | 189 ++++++++++++++++++ .../mesh-issues/cmd/mesh-issues/delivery.go | 65 ++++-- modules/mesh-issues/cmd/mesh-issues/issue.go | 3 + modules/mesh-issues/cmd/mesh-issues/main.go | 18 +- modules/mesh-issues/cmd/mesh-issues/ready.go | 25 ++- .../mesh-issues/cmd/mesh-issues/tracker.go | 28 ++- .../mesh-issues/cmd/mesh-issues/tracking.go | 2 +- modules/mesh-issues/cmd/mesh-issues/worker.go | 10 +- modules/mesh-issues/module.json | 2 +- 10 files changed, 345 insertions(+), 24 deletions(-) create mode 100644 modules/mesh-issues/cmd/mesh-issues/claims.go diff --git a/modules/mesh-issues/README.md b/modules/mesh-issues/README.md index 59a01838..7401f1fd 100644 --- a/modules/mesh-issues/README.md +++ b/modules/mesh-issues/README.md @@ -19,7 +19,7 @@ what it refuses. |---|---|---| | `open` | `title`, `report`, optionally `located-in`, `record` | checks the issue, then takes the next number and keeps it under that number; answers the number | | `get` | `number` | the issue whole: fields, notes, links | -| `list` | optionally `status`, `owner`, `phrase`, `limit` | short rows, newest first: number, status, owner, title | +| `list` | optionally `status`, `owner`, `phrase`, `limit`, `severity`, `needs`, `ready`, `flag` (`delivered-not-resolved` or `stale-claim`) | short rows, newest first: number, status, owner, title, and each flag that holds | | `move` | `number`, `status`, and what that status needs (`by` for `diagnosing` and `fixing`, optionally `take-over`) | changes the status under the development cycle's rules | | `note` | `number`, `text` | appends a note; a note is never edited | | `link` | `number`, `kind`, `target`, optionally `remove` | adds or removes a typed link | @@ -110,6 +110,27 @@ event `ready-to-verify`, once per set of fixes; it is resolved once its acceptan while the issue stays `fixing`: `move` to the status it has changes what it holds and is refused when it changes nothing. +**The fixes of an issue are its `fixed-by` and the pull requests linked to it** (link kind `pull-request`; hq issue +483): agents link a pull request while the issue is in fixing and give `fixed-by` at the resolve, so a delivery read +on `fixed-by` alone missed every fix being verified. Readiness still reads `fixed-by` alone, so a pull request linked +and abandoned holds no dependent back. A pull request linked after its delivery moved hears no transition again: each +sweep asks mesh-delivery for at most five fixes never heard of, of issues not resolved. + +The ready-to-verify note names each fix with when it was delivered, and the issue's worker; the event carries +`worker` and `delivered-at`. No channel of the tracker reaches a worker, so the note and the event are how it hears. + +**Delivered, not resolved** (hq issue 483). An issue in fixing whose every fix is delivered carries +`delivered-not-resolved` on `get` and its `list` row. Past `delivered-not-resolved-after` (default `1h`) from the last +delivery, the sweep says it once as a warning on the operator's channel, key +`issue-tracker.delivered-not-resolved.`, "issue N: fixed and delivered, not resolved; works on it", +cleared there when the issue leaves fixing or names a fix not delivered yet (at its `moved`, or the next sweep). + +**A stale claim** (hq issue 483). An issue in diagnosing or fixing whose worker did nothing on it for +`worker-stale-after` (default `24h`) and that names no pull request still open (not merged, or never heard of) +carries `stale-claim` on `get` and its `list` row, beside its `worker`. A move by someone else takes it over without +`take-over`'s reason, as any stale worker; the answer, the note and the `moved` event say the claim was stale +(`stale-claim`). + 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.`), cleared there when every fix of the issue is delivered. A module that runs only @@ -243,7 +264,9 @@ as the credential, which ADR 0266 keeps agents from reading (hq issue 333). `duplicate-check` (`on`; `off` stops the triage of new issues), `judge-model` (`sonnet`; `haiku` or `opus`) and `forge-owner` (the forge's owner of the repositories `fixed-by` names, for asking a delivery) steer the triage; -`stuck-after` (`2h`) is how long a merged fix may take to be delivered before it is said. +`stuck-after` (`2h`) is how long a merged fix may take to be delivered before it is said; +`delivered-not-resolved-after` (`1h`) how long an issue may stay in fixing once every fix is delivered before it is +said (hq issue 483). ``` { diff --git a/modules/mesh-issues/cmd/mesh-issues/claims.go b/modules/mesh-issues/cmd/mesh-issues/claims.go new file mode 100644 index 00000000..b61294e6 --- /dev/null +++ b/modules/mesh-issues/cmd/mesh-issues/claims.go @@ -0,0 +1,189 @@ +package main + +// A fix delivered and an issue left in fixing, and a claim nobody works on (novox/hq issue 483). The operator found +// 13 of 23 issues in fixing whose fix was merged and delivered hours before, and two claimed by builders that had +// stopped: the tracker measured the delivery and said "ready to verify", but nothing said an issue left there, and +// nothing told a worked issue from a forgotten one. Here: +// +// - an issue in fixing whose every fix (fixed-by and the pull requests linked) is delivered is +// delivered-not-resolved; past `delivered-not-resolved-after` (1h) it is said to the operator's channel as a +// warning, key `issue-tracker.delivered-not-resolved.`, naming the issue and its worker, cleared there +// when it leaves fixing or names a fix not yet delivered; +// - an issue in diagnosing or fixing whose worker did nothing on it for `worker-stale-after` (24h) and that names +// no pull request still open is a stale claim: get and list say so, and a move by someone else takes it over +// without a reason, saying the claim was stale. +// +// A module that runs only its bundle cannot raise a controller condition (hq ADR 0240 rule 8, ADR 0256), so the +// warning is the operator's channel's, as the stuck fix's is (ADR 0281 §4). + +import ( + "fmt" + "strings" + "time" +) + +// DefaultDeliveredNotResolvedAfter is how long an issue may stay in fixing once its fix is delivered before the +// operator hears of it: proving an acceptance live takes minutes, and a worker that went away would otherwise +// leave the issue there for good. +const DefaultDeliveredNotResolvedAfter = time.Hour + +// MostSweepAsks is the most deliveries one sweep asks of mesh-delivery, each up to the bus's 30 seconds: a fix +// linked after its delivery moved is measured within a few sweeps, and a sweep stays short. +const MostSweepAsks = 5 + +// The flags list filters on (novox/hq issue 483). +const ( + FlagDeliveredNotResolved = "delivered-not-resolved" + FlagStaleClaim = "stale-claim" +) + +// Flags is every flag list takes. +var Flags = []string{FlagDeliveredNotResolved, FlagStaleClaim} + +// deliveredNotResolvedAfter is the setting's bound, or the default. +func (t *Tracker) deliveredNotResolvedAfter() time.Duration { + if s, err := t.Settings(); err == nil && strings.TrimSpace(s.DeliveredNotResolvedAfter) != "" { + if d, err := time.ParseDuration(strings.TrimSpace(s.DeliveredNotResolvedAfter)); err == nil && d > 0 { + return d + } + } + return DefaultDeliveredNotResolvedAfter +} + +// deliveredNotResolved says whether an issue is in fixing and every fix it names is delivered. +func deliveredNotResolved(is *Issue) bool { return is.Status == StatusFixing && allDelivered(is) } + +// deliveredAt is when the last of an issue's delivered fixes was delivered, as the delivery said it. +func deliveredAt(is *Issue) time.Time { + var last time.Time + for _, pr := range fixPullRequests(is) { + if f := fixOf(is, pr); f != nil && contains(is.Delivered, pr) && f.Since.After(last) { + last = f.Since + } + } + return last +} + +func notResolvedKey(n uint64) string { return "issue-tracker.delivered-not-resolved." + keyOf(n) } + +// sayNotResolved warns the operator's channel, once per spell, of an issue delivered and not resolved past the +// bound, and answers whether it did; an issue no longer so has its warning cleared. +func (t *Tracker) sayNotResolved(is *Issue, now time.Time) (bool, error) { + if !deliveredNotResolved(is) { + return false, t.clearNotResolved(is) + } + at := deliveredAt(is) + if is.NotResolvedOpen || t.Notify == nil || now.Sub(at) < t.deliveredNotResolvedAfter() { + return false, nil + } + worker := "nobody" + if is.Worker != nil { + worker = is.Worker.By + } + ago := now.Sub(at).Round(time.Minute) + prs := strings.Join(fixPullRequests(is), ", ") + if err := t.Notify(notResolvedKey(is.Number), "warning", + fmt.Sprintf("issue %d: fixed and delivered, not resolved; %s works on it", is.Number, worker), + fmt.Sprintf("Fixed and delivered, not resolved (issue %d)", is.Number), + fmt.Sprintf("Every fix of issue %d (%s) was delivered %s ago, at %s, and the issue is still in fixing; %s works "+ + "on it. Its worker proves the acceptance live and resolves it. When the worker is gone, another agent "+ + "takes it over with a move to fixing and resolves it; nothing is needed from you otherwise.", is.Number, + prs, ago, at.Format(time.RFC3339), worker), "", false); err != nil { + return false, err + } + _, err := t.change(is.Number, func(i *Issue) error { i.NotResolvedOpen = true; return nil }) + return err == nil, err +} + +// clearNotResolved clears the warning once the issue is no longer delivered and in fixing: resolved, moved on, or +// naming a fix not delivered yet. +func (t *Tracker) clearNotResolved(is *Issue) error { + if !is.NotResolvedOpen || deliveredNotResolved(is) || t.Notify == nil { + return nil + } + if err := t.Notify(notResolvedKey(is.Number), "", "", "", "", fmt.Sprintf("issue %d left fixing (%s), or names a "+ + "fix not delivered yet", is.Number, is.Status), true); err != nil { + return err + } + _, err := t.change(is.Number, func(i *Issue) error { i.NotResolvedOpen = false; return nil }) + return err +} + +// askUnheard asks mesh-delivery for a few fixes of issues not yet resolved that the tracker never heard of: a pull +// request linked after its delivery moved hears no transition again, and OnMoved asks only at a move. +func (t *Tracker) askUnheard() error { + if t.Deliveries == nil { + return nil + } + all, err := t.allIssues() + if err != nil { + return err + } + asked := 0 + for n, is := range all { + if !unresolved(is) { + continue + } + heard := false + for _, pr := range fixPullRequests(is) { + if fixOf(is, pr) != nil { + continue + } + if asked == MostSweepAsks { + return nil + } + asked++ + repo, number, _ := strings.Cut(pr, "#") + d, err := t.Deliveries(repo, number) + if err != nil { + return fmt.Errorf("the delivery of %s (issue %d): %v", pr, n, err) + } + if d.State == "" { + continue + } + since := d.Since + if since.IsZero() { + since = t.now() + } + if _, err := t.recordFix(pr, d.State, d.Why, afterMerge(d.State), since, n); err != nil { + return err + } + heard = true + } + if heard { + if err := t.afterFix(n); err != nil { + return err + } + } + } + return nil +} + +// openPullRequest says whether an issue names a pull request not merged yet, or one whose delivery the tracker +// never heard of: work in review is work, and a fix unheard of is not taken for none. +func openPullRequest(is *Issue) bool { + for _, pr := range fixPullRequests(is) { + f := fixOf(is, pr) + if f == nil || (!f.Merged && !contains(is.Delivered, pr)) { + return true + } + } + return false +} + +// staleClaim says whether an issue's worker went quiet with no pull request open: nothing done on it for the bound, +// and nothing in review. +func staleClaim(is *Issue, now time.Time, after time.Duration) bool { + return worked(is.Status) && is.Worker != nil && is.Worker.stale(now, after) && !openPullRequest(is) +} + +// hasFlag says whether an issue carries a list flag. +func hasFlag(is *Issue, flag string, now time.Time, staleAfter time.Duration) bool { + switch flag { + case FlagDeliveredNotResolved: + return deliveredNotResolved(is) + case FlagStaleClaim: + return staleClaim(is, now, staleAfter) + } + return false +} diff --git a/modules/mesh-issues/cmd/mesh-issues/delivery.go b/modules/mesh-issues/cmd/mesh-issues/delivery.go index eaa84b82..749c8c57 100644 --- a/modules/mesh-issues/cmd/mesh-issues/delivery.go +++ b/modules/mesh-issues/cmd/mesh-issues/delivery.go @@ -64,7 +64,7 @@ func fixOf(is *Issue, pr string) *Fix { // allDelivered says whether an issue names a pull request and every one is delivered. func allDelivered(is *Issue) bool { - prs := pullRequests(is) + prs := fixPullRequests(is) if len(prs) == 0 { return false } @@ -79,7 +79,7 @@ func allDelivered(is *Issue) bool { // DeliveryLine is the derived delivery state of an issue's fix, in words; empty when fixed-by names no pull // request. func DeliveryLine(is *Issue) string { - prs := pullRequests(is) + prs := fixPullRequests(is) if len(prs) == 0 { return "" } @@ -128,7 +128,7 @@ func (t *Tracker) recordFix(pr, state, why string, merged bool, at time.Time, on } var changed []uint64 for n, is := range all { - if (only != 0 && n != only) || !contains(pullRequests(is), pr) { + if (only != 0 && n != only) || !contains(fixPullRequests(is), pr) { continue } kept := cut(why, MostReasonKept) @@ -177,17 +177,41 @@ func (t *Tracker) afterFix(n uint64) error { 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 (tried again at the next sweep): %v", n, err) } + if err := t.clearNotResolved(is); err != nil { + t.Logf("[mesh-issues] the warning that %d is delivered and not resolved could not be cleared on the operator's channel (tried again at the next sweep): %v", n, err) + } if allDelivered(is) { if unresolved(is) { - prs := pullRequests(is) + prs := fixPullRequests(is) sort.Strings(prs) if strings.Join(is.VerifyFor, ",") != strings.Join(prs, ",") { // The note first, kept once by its words; then the claim; then the event. A failure before the claim - // is tried again at the next offer, and the note is not written twice. - prefix := fmt.Sprintf("Ready to verify: every fix of this issue is delivered (%s).", strings.Join(prs, ", ")) + // is tried again at the next offer, and the note is not written twice. It names when each fix was + // delivered and who works on the issue: no channel of the tracker reaches a worker, so the note + // and the event are how the worker hears it (novox/hq issue 483). + var when []string + for _, pr := range prs { + if f := fixOf(is, pr); f != nil { + when = append(when, pr+" at "+f.Since.Format(time.RFC3339)) + } else { + when = append(when, pr) + } + } + prefix := fmt.Sprintf("Ready to verify: every fix of this issue is delivered (%s).", strings.Join(when, ", ")) + worker := "" + if is.Worker != nil { + worker = is.Worker.By + } if !noted(is, prefix) { - if _, err := t.note(GroomBy, n, prefix+" Delivered is not resolved: prove the acceptance live, then "+ - "resolve it, from fixing: move it to fixing with by first if it is not there. (novox/hq ADR 0281, ADR 0283)"); err != nil { + text := prefix + if worker != "" { + text += fmt.Sprintf(" %s works on it: prove the acceptance live and resolve it, or say what is left. "+ + "In fixing and not resolved %s after its delivery, it is said to the operator.", worker, + t.deliveredNotResolvedAfter()) + } + text += " Delivered is not resolved: prove the acceptance live, then resolve it, from fixing: move it " + + "to fixing with by first if it is not there. (novox/hq ADR 0281, ADR 0284, issue 483)" + if _, err := t.note(GroomBy, n, text); err != nil { return err } } @@ -203,7 +227,11 @@ func (t *Tracker) afterFix(n uint64) error { return err } if claimed { - t.emit(EventReadyToVerify, map[string]any{"number": n, "fixed-by": prs, "at": t.now()}) + body := map[string]any{"number": n, "fixed-by": prs, "at": t.now(), "delivered-at": deliveredAt(is)} + if worker != "" { + body["worker"] = worker + } + t.emit(EventReadyToVerify, body) } } } @@ -217,7 +245,7 @@ func stillStuck(is *Issue) bool { if is.Status == StatusWontfix { return false } - named := pullRequests(is) + named := fixPullRequests(is) for _, f := range is.Fixes { if f.StuckSaid && contains(named, f.PullRequest) && !contains(is.Delivered, f.PullRequest) { return true @@ -334,7 +362,7 @@ func (t *Tracker) OnMoved(e stdio.Envelope) error { // Each ask up to the bus's 30 seconds, under the runtime's two minutes; one that fails, or more left to // ask, has the event offered again, so a delivery never asked ends in max-deliveries, not in silence. asked := 0 - for _, pr := range pullRequests(is) { + for _, pr := range fixPullRequests(is) { if fixOf(is, pr) != nil { continue } @@ -395,8 +423,13 @@ func (t *Tracker) stuckAfter() time.Duration { } // Sweep says every fix merged and not delivered past the bound: a note on its issue and a warning on the -// operator's channel, once; it answers how many it said. Run every few minutes. +// 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. func (t *Tracker) Sweep() (int, error) { + if err := t.askUnheard(); err != nil { + t.Logf("[mesh-issues] the sweep could not ask a fix's delivery (asked again at the next sweep): %v", err) + } all, err := t.allIssues() if err != nil { return 0, err @@ -404,6 +437,12 @@ func (t *Tracker) Sweep() (int, error) { bound, now := t.stuckAfter(), t.now() said := 0 for n, is := range all { + ok, err := t.sayNotResolved(is, now) + if err != nil { + t.Logf("[mesh-issues] that %d is delivered and not resolved could not be said on the operator's channel (said at the next sweep): %v", n, err) + } 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) @@ -413,7 +452,7 @@ func (t *Tracker) Sweep() (int, error) { 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) } - named := pullRequests(is) + named := fixPullRequests(is) for _, f := range is.Fixes { if !contains(named, f.PullRequest) || contains(is.Delivered, f.PullRequest) || !f.Merged || f.MergedAt == nil || reached(f.State) || f.StuckSaid || now.Sub(*f.MergedAt) < bound { diff --git a/modules/mesh-issues/cmd/mesh-issues/issue.go b/modules/mesh-issues/cmd/mesh-issues/issue.go index 6fae032e..351625c0 100644 --- a/modules/mesh-issues/cmd/mesh-issues/issue.go +++ b/modules/mesh-issues/cmd/mesh-issues/issue.go @@ -102,6 +102,9 @@ type Issue struct { VerifyFor []string `json:"verify-for,omitempty"` // StuckOpen is the warning on the operator's channel said for a stuck fix and not yet cleared there. StuckOpen bool `json:"stuck-open,omitempty"` + // NotResolvedOpen is the warning on the operator's channel that it is fixed and delivered and not resolved, + // said and not yet cleared there (novox/hq issue 483). + NotResolvedOpen bool `json:"not-resolved-open,omitempty"` // ReadyFor is the issues it depended on when it was last said ready (ADR 0279 §3), so one spell of // readiness is said once however many dependencies let it go at one delivery. ReadyFor []uint64 `json:"ready-for,omitempty"` diff --git a/modules/mesh-issues/cmd/mesh-issues/main.go b/modules/mesh-issues/cmd/mesh-issues/main.go index 6f042df9..7e9d5bf2 100644 --- a/modules/mesh-issues/cmd/mesh-issues/main.go +++ b/modules/mesh-issues/cmd/mesh-issues/main.go @@ -47,6 +47,10 @@ type Settings struct { // WorkerStaleAfter is how long an issue's worker stays current with nothing done on it (ADR 0283 §2), as a // duration such as 24h; DefaultWorkerStaleAfter when empty. WorkerStaleAfter string `json:"worker-stale-after,omitempty"` + // DeliveredNotResolvedAfter is how long an issue may stay in fixing once every fix it names is delivered before + // the operator is warned (novox/hq issue 483), as a duration such as 1h; DefaultDeliveredNotResolvedAfter when + // empty. + DeliveredNotResolvedAfter string `json:"delivered-not-resolved-after,omitempty"` // DecisionFloor and ResearchFloor are the highest decision and research numbers taken where the tracker // cannot read, such as an open branch of hq on the forge (issue 424). Read once, when the sequence's // counter starts: its first number is one above the highest of this floor, the numbers hq's record holds @@ -369,7 +373,10 @@ func Tools(t *Tracker) []stdio.Tool { "needs none, ready. An issue is ready when everything it depends on is closed, or resolved with its fix " + "delivered. Each row says whether its fix is delivered, and, for an issue at diagnosing or fixing, who " + "works on it (worker) since when, and stale when nobody did anything on it for a day, so it may be " + - "taken over. Answers what is open in one call.", + "taken over. flag delivered-not-resolved keeps the issues in fixing whose every fix (fixed-by and the " + + "pull requests linked) is delivered; flag stale-claim the issues whose worker did nothing on them for a " + + "day with no pull request open, which a move takes over without a reason. Each row says both. Answers " + + "what is open in one call.", Input: schema(nil, map[string]any{ "status": map[string]any{"type": "string", "enum": Statuses, "description": "only issues at this status"}, "owner": text("only issues located in this owner, such as mesh-controller"), @@ -378,9 +385,10 @@ func Tools(t *Tracker) []stdio.Tool { "severity": map[string]any{"type": "string", "enum": Severities, "description": "only issues at this severity"}, "needs": map[string]any{"type": "string", "enum": NeedsKinds, "description": "only issues that need this"}, "ready": map[string]any{"type": "boolean", "description": "only issues ready (true) or waiting (false)"}, + "flag": map[string]any{"type": "string", "enum": Flags, "description": "only issues delivered and not resolved, or the stale claims"}, }), Run: func(m map[string]any) (any, error) { - a, err := readArgs("list", m, "status", "owner", "phrase", "limit", "severity", "needs", "ready") + a, err := readArgs("list", m, "status", "owner", "phrase", "limit", "severity", "needs", "ready", "flag") if err != nil { return nil, err } @@ -400,6 +408,9 @@ func Tools(t *Tracker) []stdio.Tool { if ask.Needs, err = a.str("needs"); err != nil { return nil, err } + if ask.Flag, err = a.str("flag"); err != nil { + return nil, err + } if v, given := m["ready"]; given && v != nil { r, err := a.flag("ready") if err != nil { @@ -421,7 +432,8 @@ func Tools(t *Tracker) []stdio.Tool { "development cycle's rules. Move an issue to diagnosing or fixing when you start work on it, with by: " + "who works on it and its node, as who@node (such as fix-399@g14); the issue shows it until it moves on. " + "Refused while someone else works on it, naming them, unless take-over says why (noted on the issue); a " + - "worker who did nothing on it for a day may be taken over from without a reason. located needs located-in, owners on " + + "worker who did nothing on it for a day may be taken over from without a reason, and with no pull request open " + + "the answer says the claim was a stale claim. located needs located-in, owners on " + "the registered list; resolved is reached only from fixing and needs located-in and fixed-by (the pull " + "requests or commits), and an issue in the mesh's core also replay (the replay's id, such as R361) or " + "replay-none (why no replay is possible); wontfix needs a reason. A move to the status it has changes " + diff --git a/modules/mesh-issues/cmd/mesh-issues/ready.go b/modules/mesh-issues/cmd/mesh-issues/ready.go index ed98e6bc..a6a5f3c7 100644 --- a/modules/mesh-issues/cmd/mesh-issues/ready.go +++ b/modules/mesh-issues/cmd/mesh-issues/ready.go @@ -188,6 +188,24 @@ func pullRequests(is *Issue) []string { return out } +// fixPullRequests is every pull request an issue names as its fix: those of fixed-by and those linked as +// pull-request, as `#`, lower-case. Agents link a pull request while the issue is in fixing and +// give fixed-by only at the resolve, so a delivery measured on fixed-by alone missed every fix still being +// verified (novox/hq issue 483). Readiness (done) still reads fixed-by alone: a resolved issue's fixed-by is what +// fixed it, and a pull request linked and abandoned must not hold its dependents back. +func fixPullRequests(is *Issue) []string { + out := pullRequests(is) + for _, l := range is.Links { + if l.Kind != LinkPullRequest { + continue + } + if pr := strings.ToLower(strings.TrimSpace(l.Target)); pullRequest.MatchString(pr) && !contains(out, pr) { + out = append(out, pr) + } + } + return out +} + func contains(list []string, s string) bool { for _, e := range list { if e == s { @@ -273,6 +291,9 @@ type View struct { Delivery string `json:"delivery,omitempty"` // WorkerStale says nothing was done on it by its worker, or anyone, for worker-stale-after (ADR 0283 §2). WorkerStale bool `json:"worker-stale,omitempty"` + // DeliveredNotResolved and StaleClaim are list's flags, said of this issue (novox/hq issue 483). + DeliveredNotResolved bool `json:"delivered-not-resolved,omitempty"` + StaleClaim bool `json:"stale-claim,omitempty"` } // View is `get`. @@ -285,8 +306,10 @@ func (t *Tracker) View(n uint64) (*View, error) { if is == nil { return nil, fmt.Errorf("there is no issue %d: %w", n, ErrNoIssue) } + now, staleAfter := t.now(), t.workerStaleAfter() return &View{Issue: is, NeededBy: neededBy(all, n), WaitsFor: waitsFor(all, is), Ready: isReady(all, is), - Delivery: DeliveryLine(is), WorkerStale: is.Worker != nil && is.Worker.stale(t.now(), t.workerStaleAfter())}, nil + Delivery: DeliveryLine(is), WorkerStale: is.Worker != nil && is.Worker.stale(now, staleAfter), + DeliveredNotResolved: deliveredNotResolved(is), StaleClaim: staleClaim(is, now, staleAfter)}, nil } // letGo says, for an issue that may have just let others go (its fix delivered, or wontfix), every issue it diff --git a/modules/mesh-issues/cmd/mesh-issues/tracker.go b/modules/mesh-issues/cmd/mesh-issues/tracker.go index 5122341e..3d4c899d 100644 --- a/modules/mesh-issues/cmd/mesh-issues/tracker.go +++ b/modules/mesh-issues/cmd/mesh-issues/tracker.go @@ -348,6 +348,9 @@ type Moved struct { Since *time.Time `json:"since,omitempty"` // TookOverFrom is the worker a take-over took the issue over from. TookOverFrom string `json:"took-over-from,omitempty"` + // StaleClaim says the worker taken over from had a stale claim: nothing done on it past worker-stale-after and + // no pull request open, so no reason was needed (novox/hq issue 483). + StaleClaim bool `json:"stale-claim,omitempty"` // Released is the worker whose work on it the move ended. Released string `json:"released,omitempty"` Said string `json:"said"` @@ -518,7 +521,13 @@ func (t *Tracker) move(by string, a MoveAsk) (*Moved, error) { if takeOver != "" { body["take-over"] = takeOver } - out.Said += fmt.Sprintf(" Taken over from %s; noted on the issue.", work.from) + if work.staleClaim { + out.StaleClaim, body["stale-claim"] = true, true + out.Said += fmt.Sprintf(" Taken over from %s, whose claim was a stale claim (nothing done on it for %s and no "+ + "pull request open); noted on the issue.", work.from, staleAfter) + } else { + out.Said += fmt.Sprintf(" Taken over from %s; noted on the issue.", work.from) + } } if work.released != "" { body["released"] = work.released @@ -713,6 +722,8 @@ type ListAsk struct { Needs string // Ready, when set, keeps only the issues ready (true) or not ready (false). Ready *bool + // Flag keeps only the issues delivered and not resolved, or the stale claims (novox/hq issue 483). + Flag string } // Row is one issue in a list: short enough to read in a chat. @@ -733,6 +744,10 @@ type Row struct { Worker string `json:"worker,omitempty"` Since *time.Time `json:"since,omitempty"` Stale bool `json:"stale,omitempty"` + // DeliveredNotResolved says it is in fixing with every fix it names delivered; StaleClaim that its worker + // did nothing on it for worker-stale-after and no pull request of it is open (novox/hq issue 483). + DeliveredNotResolved bool `json:"delivered-not-resolved,omitempty"` + StaleClaim bool `json:"stale-claim,omitempty"` } // Listed is the answer to a list. @@ -771,6 +786,12 @@ func (t *Tracker) List(a ListAsk) (*Listed, error) { return nil, err } } + flag := strings.TrimSpace(a.Flag) + if flag != "" { + if _, err := oneOf("flag", flag, Flags); err != nil { + return nil, err + } + } limit := a.Limit if limit <= 0 { limit = 50 @@ -799,6 +820,9 @@ func (t *Tracker) List(a ListAsk) (*Listed, error) { if needs != "" && is.Needs != needs { continue } + if flag != "" && !hasFlag(is, flag, now, staleAfter) { + continue + } ready := isReady(all, is) if a.Ready != nil && ready != *a.Ready { continue @@ -817,6 +841,8 @@ func (t *Tracker) List(a ListAsk) (*Listed, error) { since := c.Since row.Worker, row.Since, row.Stale = c.By, &since, c.stale(now, staleAfter) } + row.DeliveredNotResolved = deliveredNotResolved(is) + row.StaleClaim = staleClaim(is, now, staleAfter) out.Rows = append(out.Rows, row) } } diff --git a/modules/mesh-issues/cmd/mesh-issues/tracking.go b/modules/mesh-issues/cmd/mesh-issues/tracking.go index aaf8b9a7..64d1344d 100644 --- a/modules/mesh-issues/cmd/mesh-issues/tracking.go +++ b/modules/mesh-issues/cmd/mesh-issues/tracking.go @@ -43,7 +43,7 @@ type Tracked struct { // when fixed-by names no pull request; failed, held, merged (not delivered yet), not-merged, unheard (nothing // heard from the delivery), or delivered (every pull request it names). func FixState(is *Issue) string { - prs := pullRequests(is) + prs := fixPullRequests(is) if len(prs) == 0 { return "" } diff --git a/modules/mesh-issues/cmd/mesh-issues/worker.go b/modules/mesh-issues/cmd/mesh-issues/worker.go index 534395e8..1faf2c55 100644 --- a/modules/mesh-issues/cmd/mesh-issues/worker.go +++ b/modules/mesh-issues/cmd/mesh-issues/worker.go @@ -90,7 +90,9 @@ type workOutcome struct { took bool // a worker started, or the issue changed hands from string // the worker it was taken over from wasStale bool // the worker taken over from was stale - released string // the worker whose work ended, on a move to a status that has none + // staleClaim is a stale worker with no pull request open: a claim nobody works on (novox/hq issue 483). + staleClaim bool + released string // the worker whose work ended, on a move to a status that has none } // workerFor applies a move's worker to an issue, under ADR 0283 §1 and §2: another worker not stale is refused @@ -121,6 +123,7 @@ func workerFor(is *Issue, to, by, takeOver string, now time.Time, staleAfter tim return out, nil } out.wasStale = cur.stale(now, staleAfter) + out.staleClaim = out.wasStale && !openPullRequest(is) if !out.wasStale && takeOver == "" { return out, fmt.Errorf("%s works on issue %d (%s) since %s, last active %s. Ask them, or take it over with "+ "take-over saying why; the reason is noted on the issue. A worker who did nothing on it for %s may be "+ @@ -136,7 +139,10 @@ func workerFor(is *Issue, to, by, takeOver string, now time.Time, staleAfter tim func takeOverNote(o workOutcome, by, reason string, prev *Worker) string { s := fmt.Sprintf("Taken over by %s from %s, who worked on it since %s and was last active %s", by, o.from, prev.Since.Format(time.RFC3339), prev.Active.Format(time.RFC3339)) - if o.wasStale { + switch { + case o.staleClaim: + s += " (a stale claim: nothing done on it past the bound, and no pull request open)" + case o.wasStale: s += " (stale)" } if reason != "" { diff --git a/modules/mesh-issues/module.json b/modules/mesh-issues/module.json index 6ef3b3bc..55f0055d 100644 --- a/modules/mesh-issues/module.json +++ b/modules/mesh-issues/module.json @@ -88,7 +88,7 @@ "path": "${dir:state}/settings.json", "mode": "0600", "merge": "json", - "content": "{\n \"first-number\": 365,\n \"owners\": [\n \"hq\",\n \"mesh-catalog\",\n \"mesh-catalog modules/adwaita\",\n \"mesh-catalog modules/artifact-store-tools\",\n \"mesh-catalog modules/asus-zephyrus-g14\",\n \"mesh-catalog modules/audit-logger\",\n \"mesh-catalog modules/avahi\",\n \"mesh-catalog modules/baserow\",\n \"mesh-catalog modules/betterbird\",\n \"mesh-catalog modules/blueman\",\n \"mesh-catalog modules/bluetooth\",\n \"mesh-catalog modules/build-agent\",\n \"mesh-catalog modules/ca-trust\",\n \"mesh-catalog modules/claude-code\",\n \"mesh-catalog modules/claude-licence-manager\",\n \"mesh-catalog modules/clipmenu\",\n \"mesh-catalog modules/confluence\",\n \"mesh-catalog modules/cups\",\n \"mesh-catalog modules/dbus\",\n \"mesh-catalog modules/de-spiegel\",\n \"mesh-catalog modules/desk-channel\",\n \"mesh-catalog modules/disk-load\",\n \"mesh-catalog modules/distribution\",\n \"mesh-catalog modules/dmenu\",\n \"mesh-catalog modules/dnsmasq\",\n \"mesh-catalog modules/docker\",\n \"mesh-catalog modules/docker-compose\",\n \"mesh-catalog modules/dunst\",\n \"mesh-catalog modules/fail2ban\",\n \"mesh-catalog modules/feh\",\n \"mesh-catalog modules/flatpak\",\n \"mesh-catalog modules/fonts\",\n \"mesh-catalog modules/forticlient\",\n \"mesh-catalog modules/gitea\",\n \"mesh-catalog modules/gitlab\",\n \"mesh-catalog modules/gnome-keyring\",\n \"mesh-catalog modules/grafana\",\n \"mesh-catalog modules/hello-web\",\n \"mesh-catalog modules/home-assistant\",\n \"mesh-catalog modules/hostname\",\n \"mesh-catalog modules/i3\",\n \"mesh-catalog modules/i3status-rust\",\n \"mesh-catalog modules/icecast\",\n \"mesh-catalog modules/influxdb\",\n \"mesh-catalog modules/invoicing\",\n \"mesh-catalog modules/jira\",\n \"mesh-catalog modules/keycloak\",\n \"mesh-catalog modules/lab\",\n \"mesh-catalog modules/lemurs\",\n \"mesh-catalog modules/letta\",\n \"mesh-catalog modules/local-model-consumer\",\n \"mesh-catalog modules/localization\",\n \"mesh-catalog modules/logrotate\",\n \"mesh-catalog modules/mailu\",\n \"mesh-catalog modules/marrytts\",\n \"mesh-catalog modules/matrix\",\n \"mesh-catalog modules/memory-pressure\",\n \"mesh-catalog modules/mesh-catalog\",\n \"mesh-catalog modules/mesh-cli\",\n \"mesh-catalog modules/mesh-delivery\",\n \"mesh-catalog modules/mesh-issues\",\n \"mesh-catalog modules/mesh-vault\",\n \"mesh-catalog modules/mesh-watcher\",\n \"mesh-catalog modules/messenger\",\n \"mesh-catalog modules/minio\",\n \"mesh-catalog modules/model-usage\",\n \"mesh-catalog modules/mongodb\",\n \"mesh-catalog modules/mosquitto\",\n \"mesh-catalog modules/mounts\",\n \"mesh-catalog modules/mssql\",\n \"mesh-catalog modules/n8n\",\n \"mesh-catalog modules/nats\",\n \"mesh-catalog modules/netcheck\",\n \"mesh-catalog modules/networkmanager\",\n \"mesh-catalog modules/nextcloud\",\n \"mesh-catalog modules/nextcloud-client\",\n \"mesh-catalog modules/nfs-server\",\n \"mesh-catalog modules/nftables\",\n \"mesh-catalog modules/nm-applet\",\n \"mesh-catalog modules/node-env\",\n \"mesh-catalog modules/nodered\",\n \"mesh-catalog modules/ollama\",\n \"mesh-catalog modules/only-office\",\n \"mesh-catalog modules/openai-consumer\",\n \"mesh-catalog modules/openrazer\",\n \"mesh-catalog modules/pacman\",\n \"mesh-catalog modules/photos-eef\",\n \"mesh-catalog modules/photos-filip\",\n \"mesh-catalog modules/picom\",\n \"mesh-catalog modules/polychromatic\",\n \"mesh-catalog modules/postgres\",\n \"mesh-catalog modules/power\",\n \"mesh-catalog modules/powerlevel10k\",\n \"mesh-catalog modules/records\",\n \"mesh-catalog modules/redis\",\n \"mesh-catalog modules/restic\",\n \"mesh-catalog modules/rofi\",\n \"mesh-catalog modules/route-adapter\",\n \"mesh-catalog modules/route-proxy\",\n \"mesh-catalog modules/screen-lock\",\n \"mesh-catalog modules/searxng\",\n \"mesh-catalog modules/sensors\",\n \"mesh-catalog modules/showcase\",\n \"mesh-catalog modules/slack\",\n \"mesh-catalog modules/snapd\",\n \"mesh-catalog modules/ssh-client\",\n \"mesh-catalog modules/sshd\",\n \"mesh-catalog modules/step-ca\",\n \"mesh-catalog modules/sudo\",\n \"mesh-catalog modules/supabase\",\n \"mesh-catalog modules/systemd\",\n \"mesh-catalog modules/systemd-networkd\",\n \"mesh-catalog modules/systemd-resolved\",\n \"mesh-catalog modules/telegram\",\n \"mesh-catalog modules/time-sync\",\n \"mesh-catalog modules/triggerhappy\",\n \"mesh-catalog modules/umami\",\n \"mesh-catalog modules/unifi\",\n \"mesh-catalog modules/website\",\n \"mesh-catalog modules/xclip\",\n \"mesh-catalog modules/xdg\",\n \"mesh-catalog modules/xorg\",\n \"mesh-catalog modules/xterm\",\n \"mesh-catalog modules/zfs\",\n \"mesh-catalog modules/zsh\",\n \"mesh-catalog modules/zsh-autosuggestions\",\n \"mesh-catalog modules/zsh-syntax-highlighting\",\n \"mesh-controller\",\n \"mesh-host\",\n \"mesh-lab\",\n \"mesh-media-catalog\",\n \"mesh-sdk\",\n \"mesh-tools\"\n ],\n \"duplicate-check\": \"on\",\n \"judge-model\": \"sonnet\",\n \"forge-owner\": \"novox\",\n \"stuck-after\": \"2h\",\n \"worker-stale-after\": \"24h\",\n \"decision-floor\": 293,\n \"research-floor\": 45\n}\n" + "content": "{\n \"first-number\": 365,\n \"owners\": [\n \"hq\",\n \"mesh-catalog\",\n \"mesh-catalog modules/adwaita\",\n \"mesh-catalog modules/artifact-store-tools\",\n \"mesh-catalog modules/asus-zephyrus-g14\",\n \"mesh-catalog modules/audit-logger\",\n \"mesh-catalog modules/avahi\",\n \"mesh-catalog modules/baserow\",\n \"mesh-catalog modules/betterbird\",\n \"mesh-catalog modules/blueman\",\n \"mesh-catalog modules/bluetooth\",\n \"mesh-catalog modules/build-agent\",\n \"mesh-catalog modules/ca-trust\",\n \"mesh-catalog modules/claude-code\",\n \"mesh-catalog modules/claude-licence-manager\",\n \"mesh-catalog modules/clipmenu\",\n \"mesh-catalog modules/confluence\",\n \"mesh-catalog modules/cups\",\n \"mesh-catalog modules/dbus\",\n \"mesh-catalog modules/de-spiegel\",\n \"mesh-catalog modules/desk-channel\",\n \"mesh-catalog modules/disk-load\",\n \"mesh-catalog modules/distribution\",\n \"mesh-catalog modules/dmenu\",\n \"mesh-catalog modules/dnsmasq\",\n \"mesh-catalog modules/docker\",\n \"mesh-catalog modules/docker-compose\",\n \"mesh-catalog modules/dunst\",\n \"mesh-catalog modules/fail2ban\",\n \"mesh-catalog modules/feh\",\n \"mesh-catalog modules/flatpak\",\n \"mesh-catalog modules/fonts\",\n \"mesh-catalog modules/forticlient\",\n \"mesh-catalog modules/gitea\",\n \"mesh-catalog modules/gitlab\",\n \"mesh-catalog modules/gnome-keyring\",\n \"mesh-catalog modules/grafana\",\n \"mesh-catalog modules/hello-web\",\n \"mesh-catalog modules/home-assistant\",\n \"mesh-catalog modules/hostname\",\n \"mesh-catalog modules/i3\",\n \"mesh-catalog modules/i3status-rust\",\n \"mesh-catalog modules/icecast\",\n \"mesh-catalog modules/influxdb\",\n \"mesh-catalog modules/invoicing\",\n \"mesh-catalog modules/jira\",\n \"mesh-catalog modules/keycloak\",\n \"mesh-catalog modules/lab\",\n \"mesh-catalog modules/lemurs\",\n \"mesh-catalog modules/letta\",\n \"mesh-catalog modules/local-model-consumer\",\n \"mesh-catalog modules/localization\",\n \"mesh-catalog modules/logrotate\",\n \"mesh-catalog modules/mailu\",\n \"mesh-catalog modules/marrytts\",\n \"mesh-catalog modules/matrix\",\n \"mesh-catalog modules/memory-pressure\",\n \"mesh-catalog modules/mesh-catalog\",\n \"mesh-catalog modules/mesh-cli\",\n \"mesh-catalog modules/mesh-delivery\",\n \"mesh-catalog modules/mesh-issues\",\n \"mesh-catalog modules/mesh-vault\",\n \"mesh-catalog modules/mesh-watcher\",\n \"mesh-catalog modules/messenger\",\n \"mesh-catalog modules/minio\",\n \"mesh-catalog modules/model-usage\",\n \"mesh-catalog modules/mongodb\",\n \"mesh-catalog modules/mosquitto\",\n \"mesh-catalog modules/mounts\",\n \"mesh-catalog modules/mssql\",\n \"mesh-catalog modules/n8n\",\n \"mesh-catalog modules/nats\",\n \"mesh-catalog modules/netcheck\",\n \"mesh-catalog modules/networkmanager\",\n \"mesh-catalog modules/nextcloud\",\n \"mesh-catalog modules/nextcloud-client\",\n \"mesh-catalog modules/nfs-server\",\n \"mesh-catalog modules/nftables\",\n \"mesh-catalog modules/nm-applet\",\n \"mesh-catalog modules/node-env\",\n \"mesh-catalog modules/nodered\",\n \"mesh-catalog modules/ollama\",\n \"mesh-catalog modules/only-office\",\n \"mesh-catalog modules/openai-consumer\",\n \"mesh-catalog modules/openrazer\",\n \"mesh-catalog modules/pacman\",\n \"mesh-catalog modules/photos-eef\",\n \"mesh-catalog modules/photos-filip\",\n \"mesh-catalog modules/picom\",\n \"mesh-catalog modules/polychromatic\",\n \"mesh-catalog modules/postgres\",\n \"mesh-catalog modules/power\",\n \"mesh-catalog modules/powerlevel10k\",\n \"mesh-catalog modules/records\",\n \"mesh-catalog modules/redis\",\n \"mesh-catalog modules/restic\",\n \"mesh-catalog modules/rofi\",\n \"mesh-catalog modules/route-adapter\",\n \"mesh-catalog modules/route-proxy\",\n \"mesh-catalog modules/screen-lock\",\n \"mesh-catalog modules/searxng\",\n \"mesh-catalog modules/sensors\",\n \"mesh-catalog modules/showcase\",\n \"mesh-catalog modules/slack\",\n \"mesh-catalog modules/snapd\",\n \"mesh-catalog modules/ssh-client\",\n \"mesh-catalog modules/sshd\",\n \"mesh-catalog modules/step-ca\",\n \"mesh-catalog modules/sudo\",\n \"mesh-catalog modules/supabase\",\n \"mesh-catalog modules/systemd\",\n \"mesh-catalog modules/systemd-networkd\",\n \"mesh-catalog modules/systemd-resolved\",\n \"mesh-catalog modules/telegram\",\n \"mesh-catalog modules/time-sync\",\n \"mesh-catalog modules/triggerhappy\",\n \"mesh-catalog modules/umami\",\n \"mesh-catalog modules/unifi\",\n \"mesh-catalog modules/website\",\n \"mesh-catalog modules/xclip\",\n \"mesh-catalog modules/xdg\",\n \"mesh-catalog modules/xorg\",\n \"mesh-catalog modules/xterm\",\n \"mesh-catalog modules/zfs\",\n \"mesh-catalog modules/zsh\",\n \"mesh-catalog modules/zsh-autosuggestions\",\n \"mesh-catalog modules/zsh-syntax-highlighting\",\n \"mesh-controller\",\n \"mesh-host\",\n \"mesh-lab\",\n \"mesh-media-catalog\",\n \"mesh-sdk\",\n \"mesh-tools\"\n ],\n \"duplicate-check\": \"on\",\n \"judge-model\": \"sonnet\",\n \"forge-owner\": \"novox\",\n \"stuck-after\": \"2h\",\n \"worker-stale-after\": \"24h\",\n \"delivered-not-resolved-after\": \"1h\",\n \"decision-floor\": 293,\n \"research-floor\": 45\n}\n" } ], "build": { From 601eed06e7bfacf99acf166ff4031825bf5b858f Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 12:23:42 +0200 Subject: [PATCH 3/5] mesh-issues: test that a pull request closed unmerged is withdrawn, not stuck (hq issue 483) --- .../cmd/mesh-issues/claims_test.go | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/modules/mesh-issues/cmd/mesh-issues/claims_test.go b/modules/mesh-issues/cmd/mesh-issues/claims_test.go index aee2f4e6..2f79d92a 100644 --- a/modules/mesh-issues/cmd/mesh-issues/claims_test.go +++ b/modules/mesh-issues/cmd/mesh-issues/claims_test.go @@ -305,3 +305,103 @@ func mustView(t *testing.T, h *harness, n uint64) *View { } return v } + +// A pull request closed without merging is withdrawn: mesh-delivery says it `stopped` with no merged_as (its +// `closed` event). It is not a fix merged and stuck, not delivered, and not an open pull request: the operator is +// never told it is merged and not delivered, and a worker with nothing else in review holds a stale claim. +func TestAPullRequestClosedUnmergedIsWithdrawn(t *testing.T) { + for _, via := range []string{"transition", "checks"} { + t.Run(via, func(t *testing.T) { + h := newHarness(t) + now := clocked(h) + note := ¬ified{} + h.Notify = note.notify + n := h.open(t, "x", "mesh-catalog modules/records") + if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-x@g14"}); err != nil { + t.Fatal(err) + } + linkPR(t, h, n, "mesh-catalog#9") + closed := now.Add(-time.Hour) + if via == "transition" { + transition(t, h, 9, "ready", "its merge", "", closed.Add(-time.Hour)) + transition(t, h, 9, "stopped", "the pull request was closed without merging", "", closed) + } else { + h.Deliveries = func(string, string) (Delivery, error) { + return Delivery{State: "stopped", Why: "the pull request was closed without merging", Since: closed}, nil + } + } + *now = now.Add(25 * time.Hour) + for i := 0; i < 2; i++ { + if _, err := h.Sweep(); err != nil { + t.Fatal(err) + } + } + if len(note.said) != 0 { + t.Fatalf("a pull request closed unmerged was said to the operator: %q", note.said) + } + is := get(t, h, n) + if len(is.Fixes) != 1 || is.Fixes[0].Merged || is.Fixes[0].MergedAt != nil { + t.Fatalf("a pull request closed unmerged is kept as merged: %+v", is.Fixes) + } + v := asJSON(t, mustView(t, h, n)) + if v["delivered-not-resolved"] == true || countEvents(h, EventReadyToVerify) != 0 { + t.Fatalf("a withdrawn pull request reads as delivered: %v", v) + } + if v["stale-claim"] != true { + t.Fatalf("a withdrawn pull request reads as open, so the claim is not stale: %v", v) + } + if d, _ := v["delivery"].(string); !strings.HasPrefix(d, "fix withdrawn: mesh-catalog#9") { + t.Fatalf("delivery %q", d) + } + // A second fix linked and delivered is all the issue waits for: the withdrawn one is no fix. + linkPR(t, h, n, "mesh-catalog#10") + transition(t, h, 10, "delivered", "", "abc", *now) + if v := mustView(t, h, n); !v.DeliveredNotResolved || v.Delivery != "delivered to every machine it should reach" { + t.Fatalf("%+v %q", v.DeliveredNotResolved, v.Delivery) + } + }) + } +} + +// One pull request linked to several issues is measured on each, and each is warned and cleared on its own. +func TestOnePullRequestLinkedToSeveralIssues(t *testing.T) { + h := newHarness(t) + now := clocked(h) + note := ¬ified{} + h.Notify = note.notify + var ns []uint64 + for _, w := range []string{"fix-a@g14", "fix-b@shanks"} { + n := h.open(t, "x "+w, "mesh-catalog modules/records") + if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: w}); err != nil { + t.Fatal(err) + } + linkPR(t, h, n, "mesh-catalog#9") + ns = append(ns, n) + } + transition(t, h, 9, "delivered", "", "abc", *now) + if countEvents(h, EventReadyToVerify) != 2 { + t.Fatalf("%d ready-to-verify", countEvents(h, EventReadyToVerify)) + } + *now = now.Add(2 * time.Hour) + if _, err := h.Sweep(); err != nil { + t.Fatal(err) + } + if len(note.said) != 2 || !strings.Contains(strings.Join(note.said, "\n"), "fix-a@g14") || + !strings.Contains(strings.Join(note.said, "\n"), "fix-b@shanks") { + t.Fatalf("said %q", note.said) + } + if _, err := h.Move(MoveAsk{Number: ns[0], Status: StatusResolved, FixedBy: []string{"mesh-catalog#9"}}); err != nil { + t.Fatal(err) + } + if err := h.OnMoved(envelopeOf(ns[0])); err != nil { + t.Fatal(err) + } + if _, err := h.Sweep(); err != nil { + t.Fatal(err) + } + if len(note.clear) != 1 || note.clear[0] != notResolvedKeyOf(ns[0]) || len(note.said) != 2 { + t.Fatalf("cleared %q, said %q", note.clear, note.said) + } +} + +func notResolvedKeyOf(n uint64) string { return "issue-tracker.delivered-not-resolved." + keyOf(n) } From 3aaccba4e2475da751e42490e4d228301c6bf228 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 12:24:45 +0200 Subject: [PATCH 4/5] mesh-issues: read merged from the pull request, not the state, so a closed one is withdrawn (hq issue 483) mesh-delivery says stopped for a pull request closed unmerged too; read as merged, it was said stuck to the operator for good. Cite ADR 0284 for the worker rules, which the comments gave as 0283. --- modules/mesh-issues/README.md | 11 ++-- .../mesh-issues/cmd/mesh-issues/bus_test.go | 2 +- modules/mesh-issues/cmd/mesh-issues/claims.go | 11 ++-- .../mesh-issues/cmd/mesh-issues/delivery.go | 59 ++++++++++++++----- modules/mesh-issues/cmd/mesh-issues/groom.go | 2 +- modules/mesh-issues/cmd/mesh-issues/issue.go | 6 +- modules/mesh-issues/cmd/mesh-issues/main.go | 5 +- .../cmd/mesh-issues/memory_test.go | 2 +- modules/mesh-issues/cmd/mesh-issues/ready.go | 2 +- .../mesh-issues/cmd/mesh-issues/tracker.go | 14 ++--- .../cmd/mesh-issues/tracker_test.go | 2 +- .../mesh-issues/cmd/mesh-issues/tracking.go | 5 +- modules/mesh-issues/cmd/mesh-issues/worker.go | 6 +- .../cmd/mesh-issues/worker_test.go | 10 ++-- 14 files changed, 88 insertions(+), 49 deletions(-) diff --git a/modules/mesh-issues/README.md b/modules/mesh-issues/README.md index 7401f1fd..7e2c21e0 100644 --- a/modules/mesh-issues/README.md +++ b/modules/mesh-issues/README.md @@ -30,13 +30,13 @@ what it refuses. | `reserve` | `kind` (`decision` or `research`), optionally `for` (one line: what the record is about) | the next number of that kind, spent forever; answers `kind` and `number` (and `prefix`, the number as the record's name starts with it); see below | **The cycle's rules, at the write** (hq playbook 03, which `00-META/checks/cycle.py` enforced on the -folders until the import, and hq ADR 0283): a status is one of `open`, +folders until the import, and hq ADR 0284): a status is one of `open`, `diagnosing`, `located`, `fixing`, `resolved`, `wontfix`; `diagnosing` and `fixing` need `by`; `located` needs `located-in`; `resolved` is reached only from `fixing` and needs `located-in` and `fixed-by`; a core issue (located in the controller, the node-engine, the tool runner, the SDK, or the catalogue's bus, forge or build agent) resolves with `replay` or `replay-none`; `wontfix` needs a `reason`. A refused change changes nothing and says which rule refused it. -**Who works on it** (hq ADR 0283). A move to `diagnosing` or `fixing` names in `by` the issue's **worker**: who +**Who works on it** (hq ADR 0284). A move to `diagnosing` or `fixing` names in `by` the issue's **worker**: who works on it and its node, as `who@node` (such as `fix-399@g14` or `jochen@shanks`, kept in lower case). The issue keeps its worker (`worker`: `by`, `since`, `active`) while it stays at either status; any other status ends the work. `get` shows it, with `worker-stale`, and each `list` row shows `worker`, `since` and `stale`. A move to either @@ -113,7 +113,10 @@ nothing. **The fixes of an issue are its `fixed-by` and the pull requests linked to it** (link kind `pull-request`; hq issue 483): agents link a pull request while the issue is in fixing and give `fixed-by` at the resolve, so a delivery read on `fixed-by` alone missed every fix being verified. Readiness still reads `fixed-by` alone, so a pull request linked -and abandoned holds no dependent back. A pull request linked after its delivery moved hears no transition again: each +and abandoned holds no dependent back. A pull request closed without merging (`stopped`, and never merged) is **withdrawn**: not +a fix awaiting delivery, not delivered, not open; the line says "fix withdrawn: …" when it is the only one. Merged is +read from the delivery's `merged_as` or the pull request's own `merged`, never from the state, since `stopped` is +also what a closed pull request becomes. A pull request linked after its delivery moved hears no transition again: each sweep asks mesh-delivery for at most five fixes never heard of, of issues not resolved. The ready-to-verify note names each fix with when it was delivered, and the issue's worker; the event carries @@ -282,7 +285,7 @@ said (hq issue 483). - `decision-floor`, `research-floor` — the highest decision and research numbers taken where the tracker cannot read, such as an open branch of hq (293 and 45 when this was written: 0293 on an open branch, 045 on main). Read once, when that kind's counter starts; see *Decision and research numbers*. -- `worker-stale-after` — how long an issue's worker stays current with nothing done on the issue (hq ADR 0283), +- `worker-stale-after` — how long an issue's worker stays current with nothing done on the issue (hq ADR 0284), as a duration; `24h` when empty. - `owners` — the registered list `located-in` may name: repositories, and modules as `mesh-catalog modules/`. An entry of `located-in` is an owner, or an owner followed by a space or a diff --git a/modules/mesh-issues/cmd/mesh-issues/bus_test.go b/modules/mesh-issues/cmd/mesh-issues/bus_test.go index 5ae56806..366b386d 100644 --- a/modules/mesh-issues/cmd/mesh-issues/bus_test.go +++ b/modules/mesh-issues/cmd/mesh-issues/bus_test.go @@ -523,7 +523,7 @@ func TestAnIssueIsOpenedNotedLinkedMovedAndListedOverTheBus(t *testing.T) { var kinds []string for _, e := range events { kinds = append(kinds, e.Key) - // The move to fixing carries who works on it; every other change, who acted (ADR 0283 §4). + // The move to fixing carries who works on it; every other change, who acted (ADR 0284 §6). want := Unproven if e.Key == EventMoved && e.Body["to"] == StatusFixing { want = "fix-365@g14" diff --git a/modules/mesh-issues/cmd/mesh-issues/claims.go b/modules/mesh-issues/cmd/mesh-issues/claims.go index b61294e6..e53427f2 100644 --- a/modules/mesh-issues/cmd/mesh-issues/claims.go +++ b/modules/mesh-issues/cmd/mesh-issues/claims.go @@ -56,7 +56,7 @@ func deliveredNotResolved(is *Issue) bool { return is.Status == StatusFixing && // deliveredAt is when the last of an issue's delivered fixes was delivered, as the delivery said it. func deliveredAt(is *Issue) time.Time { var last time.Time - for _, pr := range fixPullRequests(is) { + for _, pr := range liveFixes(is) { if f := fixOf(is, pr); f != nil && contains(is.Delivered, pr) && f.Since.After(last) { last = f.Since } @@ -81,7 +81,7 @@ func (t *Tracker) sayNotResolved(is *Issue, now time.Time) (bool, error) { worker = is.Worker.By } ago := now.Sub(at).Round(time.Minute) - prs := strings.Join(fixPullRequests(is), ", ") + prs := strings.Join(liveFixes(is), ", ") if err := t.Notify(notResolvedKey(is.Number), "warning", fmt.Sprintf("issue %d: fixed and delivered, not resolved; %s works on it", is.Number, worker), fmt.Sprintf("Fixed and delivered, not resolved (issue %d)", is.Number), @@ -145,7 +145,7 @@ func (t *Tracker) askUnheard() error { if since.IsZero() { since = t.now() } - if _, err := t.recordFix(pr, d.State, d.Why, afterMerge(d.State), since, n); err != nil { + if _, err := t.recordFix(pr, d.State, d.Why, d.Merged, since, n); err != nil { return err } heard = true @@ -160,9 +160,10 @@ func (t *Tracker) askUnheard() error { } // openPullRequest says whether an issue names a pull request not merged yet, or one whose delivery the tracker -// never heard of: work in review is work, and a fix unheard of is not taken for none. +// never heard of: work in review is work, and a fix unheard of is not taken for none. One closed without merging +// is not open. func openPullRequest(is *Issue) bool { - for _, pr := range fixPullRequests(is) { + for _, pr := range liveFixes(is) { f := fixOf(is, pr) if f == nil || (!f.Merged && !contains(is.Delivered, pr)) { return true diff --git a/modules/mesh-issues/cmd/mesh-issues/delivery.go b/modules/mesh-issues/cmd/mesh-issues/delivery.go index 749c8c57..d56c73f0 100644 --- a/modules/mesh-issues/cmd/mesh-issues/delivery.go +++ b/modules/mesh-issues/cmd/mesh-issues/delivery.go @@ -44,13 +44,36 @@ func reached(state string) bool { return state == "delivered" || state == "covered" || state == "retired" } -// afterMerge says whether a state is one a delivery reaches only once merged. -func afterMerge(state string) bool { - switch state { - case "published", "delivering", "held", "delivered", "failed", "stopped", "covered", "retired": - return true +// Whether a pull request is merged is what the delivery says of the pull request itself — `merged_as` on a +// transition, the pull request's `merged` in checks — never read from the state: `stopped` is also what a pull +// request closed without merging becomes (mesh-delivery's `closed` event), and read as merged it was said stuck to +// the operator for good (novox/hq issue 483). + +// withdrawn says whether a fix is a pull request closed without merging: no fix at all, neither awaiting delivery, +// delivered, nor open. +func withdrawn(f *Fix) bool { return f != nil && f.State == "stopped" && !f.Merged } + +// liveFixes is an issue's fixes less those withdrawn: what its delivery, readiness to verify, the stuck sweep and a +// stale claim are measured on. fixPullRequests still names the withdrawn ones, so a pull request reopened is heard. +func liveFixes(is *Issue) []string { + var out []string + for _, pr := range fixPullRequests(is) { + if !withdrawn(fixOf(is, pr)) { + out = append(out, pr) + } } - return false + return out +} + +// withdrawnFixes is the pull requests of an issue closed without merging. +func withdrawnFixes(is *Issue) []string { + var out []string + for _, pr := range fixPullRequests(is) { + if withdrawn(fixOf(is, pr)) { + out = append(out, pr) + } + } + return out } func fixOf(is *Issue, pr string) *Fix { @@ -64,7 +87,7 @@ func fixOf(is *Issue, pr string) *Fix { // allDelivered says whether an issue names a pull request and every one is delivered. func allDelivered(is *Issue) bool { - prs := fixPullRequests(is) + prs := liveFixes(is) if len(prs) == 0 { return false } @@ -79,8 +102,11 @@ func allDelivered(is *Issue) bool { // DeliveryLine is the derived delivery state of an issue's fix, in words; empty when fixed-by names no pull // request. func DeliveryLine(is *Issue) string { - prs := fixPullRequests(is) + prs := liveFixes(is) if len(prs) == 0 { + if w := withdrawnFixes(is); len(w) > 0 { + return "fix withdrawn: " + strings.Join(w, ", ") + " closed without merging" + } return "" } var held, failed, merged, open, unheard []string @@ -182,7 +208,7 @@ func (t *Tracker) afterFix(n uint64) error { } if allDelivered(is) { if unresolved(is) { - prs := fixPullRequests(is) + prs := liveFixes(is) sort.Strings(prs) if strings.Join(is.VerifyFor, ",") != strings.Join(prs, ",") { // The note first, kept once by its words; then the claim; then the event. A failure before the claim @@ -245,7 +271,7 @@ func stillStuck(is *Issue) bool { if is.Status == StatusWontfix { return false } - named := fixPullRequests(is) + named := liveFixes(is) for _, f := range is.Fixes { if f.StuckSaid && contains(named, f.PullRequest) && !contains(is.Delivered, f.PullRequest) { return true @@ -296,7 +322,7 @@ func (t *Tracker) OnTransition(e stdio.Envelope) error { if at.IsZero() { at = t.now() } - merged := body.MergedAs != "" || afterMerge(body.To) + merged := body.MergedAs != "" changed, err := t.recordFix(prKey(body.Repository, body.Number), body.To, body.Why, merged, at, 0) if err != nil { return err @@ -315,6 +341,8 @@ type Delivery struct { Why string // Since is when the delivery entered its state, as it says; zero when it does not. Since time.Time + // Merged is the pull request's own merged flag, as the forge says it. + Merged bool } // DeliveryState asks mesh-delivery for one pull request's delivery. @@ -333,11 +361,14 @@ func busDeliveryState(owner string) DeliveryState { Waits string `json:"waits"` Since time.Time `json:"since"` } `json:"delivery"` + Pull struct { + Merged bool `json:"merged"` + } `json:"pull"` } if err := json.Unmarshal(raw, &out); err != nil { return Delivery{}, fmt.Errorf("mesh-delivery's checks answered no delivery: %.200s", raw) } - return Delivery{State: out.Delivery.State, Why: out.Delivery.Waits, Since: out.Delivery.Since.UTC()}, nil + return Delivery{State: out.Delivery.State, Why: out.Delivery.Waits, Since: out.Delivery.Since.UTC(), Merged: out.Pull.Merged}, nil } } @@ -386,7 +417,7 @@ func (t *Tracker) OnMoved(e stdio.Envelope) error { if since.IsZero() { since = t.now() } - if _, err := t.recordFix(pr, state, d.Why, afterMerge(state), since, body.Number); err != nil { + if _, err := t.recordFix(pr, state, d.Why, d.Merged, since, body.Number); err != nil { return err } } @@ -452,7 +483,7 @@ func (t *Tracker) Sweep() (int, error) { 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) } - named := fixPullRequests(is) + named := liveFixes(is) for _, f := range is.Fixes { if !contains(named, f.PullRequest) || contains(is.Delivered, f.PullRequest) || !f.Merged || f.MergedAt == nil || reached(f.State) || f.StuckSaid || now.Sub(*f.MergedAt) < bound { diff --git a/modules/mesh-issues/cmd/mesh-issues/groom.go b/modules/mesh-issues/cmd/mesh-issues/groom.go index 88b00a0d..bf2b2877 100644 --- a/modules/mesh-issues/cmd/mesh-issues/groom.go +++ b/modules/mesh-issues/cmd/mesh-issues/groom.go @@ -125,7 +125,7 @@ type Verdict struct { func (v Verdict) closes() bool { return v.Verdict == VerdictDuplicate && v.Certain } // unresolved says whether an issue may be offered, linked or noted by the triage: at open, diagnosing, located or -// fixing (ADR 0283 §5). +// fixing (ADR 0284 §7). func unresolved(is *Issue) bool { return is.Status == StatusOpen || is.Status == StatusDiagnosing || is.Status == StatusLocated || is.Status == StatusFixing } diff --git a/modules/mesh-issues/cmd/mesh-issues/issue.go b/modules/mesh-issues/cmd/mesh-issues/issue.go index 351625c0..4a9e8b01 100644 --- a/modules/mesh-issues/cmd/mesh-issues/issue.go +++ b/modules/mesh-issues/cmd/mesh-issues/issue.go @@ -21,9 +21,9 @@ const ( IssueCounter = "issue" ) -// The six statuses of an issue, as the development cycle names them (hq playbook 03, ADR 0275, ADR 0283). +// The six statuses of an issue, as the development cycle names them (hq playbook 03, ADR 0275, ADR 0284). // Fixing is the status an agent or person moves an issue to when they start its fix; resolved is reached -// only from it (ADR 0283 §3). +// only from it (ADR 0284 §5). const ( StatusOpen = "open" StatusDiagnosing = "diagnosing" @@ -110,7 +110,7 @@ type Issue struct { ReadyFor []uint64 `json:"ready-for,omitempty"` // Triage is the triage's judgement as it was made, kept before it is applied. Triage *Verdict `json:"triage,omitempty"` - // Worker is who works on it, while it is diagnosing or fixing (ADR 0283); nil at every other status. + // Worker is who works on it, while it is diagnosing or fixing (ADR 0284); nil at every other status. Worker *Worker `json:"worker,omitempty"` // Opening is the id of the call that opened it, so a write whose answer was lost is known as its own. Opening string `json:"opening"` diff --git a/modules/mesh-issues/cmd/mesh-issues/main.go b/modules/mesh-issues/cmd/mesh-issues/main.go index 7e9d5bf2..c5e7e8e1 100644 --- a/modules/mesh-issues/cmd/mesh-issues/main.go +++ b/modules/mesh-issues/cmd/mesh-issues/main.go @@ -44,7 +44,7 @@ type Settings struct { // StuckAfter is how long a fix may be merged and not delivered before it is said stuck (ADR 0281), as a // duration such as 2h; DefaultStuckAfter when empty. StuckAfter string `json:"stuck-after,omitempty"` - // WorkerStaleAfter is how long an issue's worker stays current with nothing done on it (ADR 0283 §2), as a + // WorkerStaleAfter is how long an issue's worker stays current with nothing done on it (ADR 0284 §4), as a // duration such as 24h; DefaultWorkerStaleAfter when empty. WorkerStaleAfter string `json:"worker-stale-after,omitempty"` // DeliveredNotResolvedAfter is how long an issue may stay in fixing once every fix it names is delivered before @@ -626,7 +626,8 @@ func Tools(t *Tracker) []stdio.Tool { Description: "Which issues track which conditions: for every condition key an issue links (link kind condition), " + "or only the keys given, the issues that link it, those not closed (resolved or wontfix) first, each with its status, " + "severity, what it needs, who works on it since when and whether they went quiet (stale), and where its " + - "fix is (fix: not-merged, merged, held, failed, unheard or delivered; empty when none is named). The " + + "fix is (fix: not-merged, merged, held, failed, unheard, delivered, or withdrawn when every pull request named was " + + "closed without merging; empty when none is named). The " + "operator's channel asks it to say in a condition's message which issue tracks it. Refused: a key not of a condition " + "key's shape.", Input: schema(nil, map[string]any{ diff --git a/modules/mesh-issues/cmd/mesh-issues/memory_test.go b/modules/mesh-issues/cmd/mesh-issues/memory_test.go index 523f90ff..f0d663ad 100644 --- a/modules/mesh-issues/cmd/mesh-issues/memory_test.go +++ b/modules/mesh-issues/cmd/mesh-issues/memory_test.go @@ -203,7 +203,7 @@ func (h *harness) open(t *testing.T, title string, located ...string) uint64 { return o.Number } -// fixing moves an issue to fixing, as the agent that starts its fix does (ADR 0283). +// fixing moves an issue to fixing, as the agent that starts its fix does (ADR 0284). func (h *harness) fixing(t *testing.T, n uint64) { t.Helper() if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix@g14"}); err != nil { diff --git a/modules/mesh-issues/cmd/mesh-issues/ready.go b/modules/mesh-issues/cmd/mesh-issues/ready.go index a6a5f3c7..2a0ff76b 100644 --- a/modules/mesh-issues/cmd/mesh-issues/ready.go +++ b/modules/mesh-issues/cmd/mesh-issues/ready.go @@ -289,7 +289,7 @@ type View struct { Ready bool `json:"ready"` // Delivery is whether its fix is delivered, in words (ADR 0281). Delivery string `json:"delivery,omitempty"` - // WorkerStale says nothing was done on it by its worker, or anyone, for worker-stale-after (ADR 0283 §2). + // WorkerStale says nothing was done on it by its worker, or anyone, for worker-stale-after (ADR 0284 §4). WorkerStale bool `json:"worker-stale,omitempty"` // DeliveredNotResolved and StaleClaim are list's flags, said of this issue (novox/hq issue 483). DeliveredNotResolved bool `json:"delivered-not-resolved,omitempty"` diff --git a/modules/mesh-issues/cmd/mesh-issues/tracker.go b/modules/mesh-issues/cmd/mesh-issues/tracker.go index 3d4c899d..4720652c 100644 --- a/modules/mesh-issues/cmd/mesh-issues/tracker.go +++ b/modules/mesh-issues/cmd/mesh-issues/tracker.go @@ -330,9 +330,9 @@ type MoveAsk struct { Replay string ReplayNone string Reason string - // By is who works on it, who@node: what a move to diagnosing or fixing needs (ADR 0283 §1). + // By is who works on it, who@node: what a move to diagnosing or fixing needs (ADR 0284 §2). By string - // TakeOver is why the issue is taken over from another worker (ADR 0283 §1). + // TakeOver is why the issue is taken over from another worker (ADR 0284 §3). TakeOver string } @@ -422,7 +422,7 @@ func (t *Tracker) move(by string, a MoveAsk) (*Moved, error) { if reason != "" && to != StatusWontfix { return nil, fmt.Errorf("reason is what wontfix needs; this moves to %s", to) } - // Who works on it (ADR 0283 §1): named on a move to diagnosing or fixing, and on no other. + // Who works on it (ADR 0284 §2): named on a move to diagnosing or fixing, and on no other. var worker, takeOver string if worked(to) { if strings.TrimSpace(a.By) == "" { @@ -507,7 +507,7 @@ func (t *Tracker) move(by string, a MoveAsk) (*Moved, error) { out := &Moved{Number: is.Number, From: from, To: to, Changed: changed, By: by, TookOverFrom: work.from, Released: work.released, Said: fmt.Sprintf("Issue %d moved from %s to %s.", is.Number, from, to)} // The event's by is who works on it when the move names them, and who acted otherwise; caller is always who - // acted, as the runtime proves it (ADR 0283 §4). + // acted, as the runtime proves it (ADR 0284 §6). body := map[string]any{"number": is.Number, "from": from, "to": to, "by": by, "caller": by, "at": t.now()} if is.Worker != nil { since := is.Worker.Since @@ -553,7 +553,7 @@ func (t *Tracker) move(by string, a MoveAsk) (*Moved, error) { } // cycleRules is what the development cycle asks of an issue moving from one status to another (playbook 03, -// to-be 53 §5, ADR 0283), said as the rule that refused it. +// to-be 53 §5, ADR 0284), said as the rule that refused it. func cycleRules(is *Issue, from, to string) error { switch to { case StatusLocated: @@ -562,7 +562,7 @@ func cycleRules(is *Issue, from, to string) error { "(a repository, or \"mesh-catalog modules/\")", is.Number) } case StatusResolved: - // Resolved only from fixing: whoever fixed it said so when they started (ADR 0283 §3). A resolved issue + // Resolved only from fixing: whoever fixed it said so when they started (ADR 0284 §5). A resolved issue // may still have what it holds corrected. if from != StatusFixing && from != StatusResolved { return fmt.Errorf("issue %d is %s, and an issue is resolved only from fixing: move it to fixing with by, "+ @@ -740,7 +740,7 @@ type Row struct { // Delivery is whether its fix is delivered, in words (ADR 0281); empty when fixed-by names no pull request. Delivery string `json:"delivery,omitempty"` // Worker is who works on it and Since since when, while it is diagnosing or fixing; Stale says nobody did - // anything on it for worker-stale-after, so it may be taken over without a reason (ADR 0283). + // anything on it for worker-stale-after, so it may be taken over without a reason (ADR 0284 §4). Worker string `json:"worker,omitempty"` Since *time.Time `json:"since,omitempty"` Stale bool `json:"stale,omitempty"` diff --git a/modules/mesh-issues/cmd/mesh-issues/tracker_test.go b/modules/mesh-issues/cmd/mesh-issues/tracker_test.go index 1308826b..557951d5 100644 --- a/modules/mesh-issues/cmd/mesh-issues/tracker_test.go +++ b/modules/mesh-issues/cmd/mesh-issues/tracker_test.go @@ -259,7 +259,7 @@ func TestAnIssueWithAFolderIsNumberedTitledAndPointsAtItsFolder(t *testing.T) { } } -// The cycle's rules at the write (to-be 53 §5, ADR 0283): one case per row, each refused case changing nothing. +// The cycle's rules at the write (to-be 53 §5, ADR 0284): one case per row, each refused case changing nothing. // A case starts from open, or, with from, from that status reached first (fixing by fix@g14). func TestTheCyclesRulesHoldAtTheWrite(t *testing.T) { const fx = StatusFixing diff --git a/modules/mesh-issues/cmd/mesh-issues/tracking.go b/modules/mesh-issues/cmd/mesh-issues/tracking.go index 64d1344d..28418332 100644 --- a/modules/mesh-issues/cmd/mesh-issues/tracking.go +++ b/modules/mesh-issues/cmd/mesh-issues/tracking.go @@ -43,8 +43,11 @@ type Tracked struct { // when fixed-by names no pull request; failed, held, merged (not delivered yet), not-merged, unheard (nothing // heard from the delivery), or delivered (every pull request it names). func FixState(is *Issue) string { - prs := fixPullRequests(is) + prs := liveFixes(is) if len(prs) == 0 { + if len(withdrawnFixes(is)) > 0 { + return "withdrawn" + } return "" } var failed, held, merged, open, unheard bool diff --git a/modules/mesh-issues/cmd/mesh-issues/worker.go b/modules/mesh-issues/cmd/mesh-issues/worker.go index 1faf2c55..61d58cbb 100644 --- a/modules/mesh-issues/cmd/mesh-issues/worker.go +++ b/modules/mesh-issues/cmd/mesh-issues/worker.go @@ -1,6 +1,6 @@ package main -// Who works on an issue (novox/hq ADR 0283). A move to diagnosing or fixing names, in `by`, the issue's worker: +// Who works on an issue (novox/hq ADR 0284). A move to diagnosing or fixing names, in `by`, the issue's worker: // the agent or person and its node, as who@node. The issue keeps its worker while it stays at either status, and // `get` and `list` show it with since. A move to either by someone else while the worker is not stale is refused, // naming the worker; a take-over gives a reason, and the reason is noted. A worker with no move, note, link or set @@ -95,7 +95,7 @@ type workOutcome struct { released string // the worker whose work ended, on a move to a status that has none } -// workerFor applies a move's worker to an issue, under ADR 0283 §1 and §2: another worker not stale is refused +// workerFor applies a move's worker to an issue, under ADR 0284 §3 and §4: another worker not stale is refused // without a take-over reason; a take-over reason with no other worker to take over from is refused. func workerFor(is *Issue, to, by, takeOver string, now time.Time, staleAfter time.Duration) (workOutcome, error) { var out workOutcome @@ -148,5 +148,5 @@ func takeOverNote(o workOutcome, by, reason string, prev *Worker) string { if reason != "" { s += ". Why: " + reason } - return s + ". (novox/hq ADR 0283)" + return s + ". (novox/hq ADR 0284)" } diff --git a/modules/mesh-issues/cmd/mesh-issues/worker_test.go b/modules/mesh-issues/cmd/mesh-issues/worker_test.go index be814aea..c1e58bbc 100644 --- a/modules/mesh-issues/cmd/mesh-issues/worker_test.go +++ b/modules/mesh-issues/cmd/mesh-issues/worker_test.go @@ -15,7 +15,7 @@ func clocked(h *harness) *time.Time { } // A move to diagnosing or fixing by someone else while the worker is current is refused, naming the worker, and -// changes nothing (ADR 0283 §1). +// changes nothing (ADR 0284 §3). func TestAnotherWorkerIsRefusedNamingTheWorker(t *testing.T) { for _, to := range []string{StatusDiagnosing, StatusFixing} { t.Run(to, func(t *testing.T) { @@ -76,7 +76,7 @@ func TestTheWorkerMovesOn(t *testing.T) { } } -// A take-over gives a reason, the reason is noted, and the moved event says from whom (ADR 0283 §1, §4). +// A take-over gives a reason, the reason is noted, and the moved event says from whom (ADR 0284 §3, §6). func TestATakeOverNeedsAReasonAndIsNoted(t *testing.T) { h := newHarness(t) clocked(h) @@ -103,7 +103,7 @@ func TestATakeOverNeedsAReasonAndIsNoted(t *testing.T) { } // A worker with nothing done on it past worker-stale-after is stale: list and get say so, and it may be taken over -// without a reason, noted (ADR 0283 §2). A caller's note keeps it live; the tracker's own does not. +// without a reason, noted (ADR 0284 §4). A caller's note keeps it live; the tracker's own does not. func TestAStaleWorkerIsShownAndMayBeTakenOverWithoutAReason(t *testing.T) { h := newHarness(t) now := clocked(h) @@ -193,7 +193,7 @@ func TestAMoveOnEndsTheWork(t *testing.T) { } } -// The duplicate check and the triage treat fixing as not closed, like diagnosing and located (ADR 0283 §5). +// The duplicate check and the triage treat fixing as not closed, like diagnosing and located (ADR 0284 §7). func TestFixingIsNotClosed(t *testing.T) { for _, s := range Statuses { want := s != StatusResolved && s != StatusWontfix @@ -261,7 +261,7 @@ func TestAResolvedIssueMayBeCorrected(t *testing.T) { } } -// A caller's link and set keep the worker current, as a note does (ADR 0283 §4). +// A caller's link and set keep the worker current, as a note does (ADR 0284 §4). func TestALinkAndASetKeepTheWorkerCurrent(t *testing.T) { for _, act := range []string{"link", "unlink", "set"} { t.Run(act, func(t *testing.T) { From d6148a5ba7971e7380a6ee2579f690d442ec678c Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 11 Oct 2026 12:26:57 +0200 Subject: [PATCH 5/5] mesh-issues: call an issue its worker left abandoned, not a stale claim (hq ADR 0294) The glossary keeps claim for a module's on a seat. --- modules/mesh-issues/README.md | 10 +++--- modules/mesh-issues/cmd/mesh-issues/claims.go | 18 +++++----- .../cmd/mesh-issues/claims_test.go | 36 +++++++++---------- .../mesh-issues/cmd/mesh-issues/delivery.go | 2 +- modules/mesh-issues/cmd/mesh-issues/main.go | 6 ++-- modules/mesh-issues/cmd/mesh-issues/ready.go | 6 ++-- .../mesh-issues/cmd/mesh-issues/tracker.go | 18 +++++----- modules/mesh-issues/cmd/mesh-issues/worker.go | 12 +++---- 8 files changed, 54 insertions(+), 54 deletions(-) diff --git a/modules/mesh-issues/README.md b/modules/mesh-issues/README.md index 7e2c21e0..082f4dac 100644 --- a/modules/mesh-issues/README.md +++ b/modules/mesh-issues/README.md @@ -19,7 +19,7 @@ what it refuses. |---|---|---| | `open` | `title`, `report`, optionally `located-in`, `record` | checks the issue, then takes the next number and keeps it under that number; answers the number | | `get` | `number` | the issue whole: fields, notes, links | -| `list` | optionally `status`, `owner`, `phrase`, `limit`, `severity`, `needs`, `ready`, `flag` (`delivered-not-resolved` or `stale-claim`) | short rows, newest first: number, status, owner, title, and each flag that holds | +| `list` | optionally `status`, `owner`, `phrase`, `limit`, `severity`, `needs`, `ready`, `flag` (`delivered-not-resolved` or `abandoned`) | short rows, newest first: number, status, owner, title, and each flag that holds | | `move` | `number`, `status`, and what that status needs (`by` for `diagnosing` and `fixing`, optionally `take-over`) | changes the status under the development cycle's rules | | `note` | `number`, `text` | appends a note; a note is never edited | | `link` | `number`, `kind`, `target`, optionally `remove` | adds or removes a typed link | @@ -128,11 +128,11 @@ delivery, the sweep says it once as a warning on the operator's channel, key `issue-tracker.delivered-not-resolved.`, "issue N: fixed and delivered, not resolved; works on it", cleared there when the issue leaves fixing or names a fix not delivered yet (at its `moved`, or the next sweep). -**A stale claim** (hq issue 483). An issue in diagnosing or fixing whose worker did nothing on it for +**Abandoned** (hq issue 483). An issue in diagnosing or fixing whose worker did nothing on it for `worker-stale-after` (default `24h`) and that names no pull request still open (not merged, or never heard of) -carries `stale-claim` on `get` and its `list` row, beside its `worker`. A move by someone else takes it over without -`take-over`'s reason, as any stale worker; the answer, the note and the `moved` event say the claim was stale -(`stale-claim`). +carries `abandoned` on `get` and its `list` row, beside its `worker`. A move by someone else takes it over without +`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 diff --git a/modules/mesh-issues/cmd/mesh-issues/claims.go b/modules/mesh-issues/cmd/mesh-issues/claims.go index e53427f2..03aae0f5 100644 --- a/modules/mesh-issues/cmd/mesh-issues/claims.go +++ b/modules/mesh-issues/cmd/mesh-issues/claims.go @@ -1,6 +1,6 @@ package main -// A fix delivered and an issue left in fixing, and a claim nobody works on (novox/hq issue 483). The operator found +// A fix delivered and an issue left in fixing, and an issue its worker abandoned (novox/hq issue 483). The operator found // 13 of 23 issues in fixing whose fix was merged and delivered hours before, and two claimed by builders that had // stopped: the tracker measured the delivery and said "ready to verify", but nothing said an issue left there, and // nothing told a worked issue from a forgotten one. Here: @@ -10,8 +10,8 @@ package main // warning, key `issue-tracker.delivered-not-resolved.`, naming the issue and its worker, cleared there // when it leaves fixing or names a fix not yet delivered; // - an issue in diagnosing or fixing whose worker did nothing on it for `worker-stale-after` (24h) and that names -// no pull request still open is a stale claim: get and list say so, and a move by someone else takes it over -// without a reason, saying the claim was stale. +// no pull request still open is abandoned (the glossary keeps "claim" for a module's on a seat): get and list +// say so, and a move by someone else takes it over without a reason, saying it was abandoned. // // A module that runs only its bundle cannot raise a controller condition (hq ADR 0240 rule 8, ADR 0256), so the // warning is the operator's channel's, as the stuck fix's is (ADR 0281 §4). @@ -34,11 +34,11 @@ const MostSweepAsks = 5 // The flags list filters on (novox/hq issue 483). const ( FlagDeliveredNotResolved = "delivered-not-resolved" - FlagStaleClaim = "stale-claim" + FlagAbandoned = "abandoned" ) // Flags is every flag list takes. -var Flags = []string{FlagDeliveredNotResolved, FlagStaleClaim} +var Flags = []string{FlagDeliveredNotResolved, FlagAbandoned} // deliveredNotResolvedAfter is the setting's bound, or the default. func (t *Tracker) deliveredNotResolvedAfter() time.Duration { @@ -172,9 +172,9 @@ func openPullRequest(is *Issue) bool { return false } -// staleClaim says whether an issue's worker went quiet with no pull request open: nothing done on it for the bound, +// abandoned says whether an issue's worker went quiet with no pull request open: nothing done on it for the bound, // and nothing in review. -func staleClaim(is *Issue, now time.Time, after time.Duration) bool { +func abandoned(is *Issue, now time.Time, after time.Duration) bool { return worked(is.Status) && is.Worker != nil && is.Worker.stale(now, after) && !openPullRequest(is) } @@ -183,8 +183,8 @@ func hasFlag(is *Issue, flag string, now time.Time, staleAfter time.Duration) bo switch flag { case FlagDeliveredNotResolved: return deliveredNotResolved(is) - case FlagStaleClaim: - return staleClaim(is, now, staleAfter) + case FlagAbandoned: + return abandoned(is, now, staleAfter) } return false } diff --git a/modules/mesh-issues/cmd/mesh-issues/claims_test.go b/modules/mesh-issues/cmd/mesh-issues/claims_test.go index 2f79d92a..7a627b9f 100644 --- a/modules/mesh-issues/cmd/mesh-issues/claims_test.go +++ b/modules/mesh-issues/cmd/mesh-issues/claims_test.go @@ -205,9 +205,9 @@ func TestDeliveredNotResolvedIsRetriedAndClearedAtTheSweep(t *testing.T) { } } -// Part 3: an issue in fixing with no open linked pull request and nothing done on it for a day is a stale claim, -// naming its worker; a new worker takes it over without a reason, and the tracker says the claim was stale. -func TestAStaleClaimIsFlaggedAndTakenOverWithoutAReason(t *testing.T) { +// Part 3: an issue in fixing with no open linked pull request and nothing done on it for a day is an abandoned issue, +// naming its worker; a new worker takes it over without a reason, and the tracker says it was abandoned. +func TestAnAbandonedIssueIsFlaggedAndTakenOverWithoutAReason(t *testing.T) { h := newHarness(t) now := clocked(h) n := h.open(t, "claimed and left", "hq") @@ -221,35 +221,35 @@ func TestAStaleClaimIsFlaggedAndTakenOverWithoutAReason(t *testing.T) { linkPR(t, h, busy, "mesh-catalog#9") transition(t, h, 9, "ready", "its merge", "", *now) *now = now.Add(23 * time.Hour) - if rows := rowsOf(t, h, map[string]any{"flag": "stale-claim"}); len(rows) != 0 { + if rows := rowsOf(t, h, map[string]any{"flag": "abandoned"}); len(rows) != 0 { t.Fatalf("stale before the day: %v", rows) } *now = now.Add(2 * time.Hour) - rows := rowsOf(t, h, map[string]any{"flag": "stale-claim"}) - if len(rows) != 1 || rows[0]["number"] != float64(n) || rows[0]["worker"] != "build-424@shanks" || rows[0]["stale-claim"] != true { + rows := rowsOf(t, h, map[string]any{"flag": "abandoned"}) + if len(rows) != 1 || rows[0]["number"] != float64(n) || rows[0]["worker"] != "build-424@shanks" || rows[0]["abandoned"] != true { t.Fatalf("want only %d flagged naming its worker, got %v", n, rows) } - if v := asJSON(t, mustView(t, h, n)); v["stale-claim"] != true { + if v := asJSON(t, mustView(t, h, n)); v["abandoned"] != true { t.Fatalf("get does not say it: %v", v) } - if v := asJSON(t, mustView(t, h, busy)); v["stale-claim"] == true { + if v := asJSON(t, mustView(t, h, busy)); v["abandoned"] == true { t.Fatal("an issue with an open pull request is flagged") } m, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-483@shanks"}) if err != nil { - t.Fatalf("a stale claim was not taken over without a reason: %v", err) + t.Fatalf("an abandoned issue was not taken over without a reason: %v", err) } - if a := asJSON(t, m); a["stale-claim"] != true || !strings.Contains(m.Said, "stale claim") { - t.Fatalf("the answer does not say the claim was stale: %v", a) + if a := asJSON(t, m); a["abandoned"] != true || !strings.Contains(m.Said, "abandoned") { + t.Fatalf("the answer does not say it was abandoned: %v", a) } is := get(t, h, n) last := is.Notes[len(is.Notes)-1].Text - if !strings.Contains(last, "stale claim") || !strings.Contains(last, "build-424@shanks") { - t.Fatalf("the note does not say the claim was stale: %s", last) + if !strings.Contains(last, "abandoned") || !strings.Contains(last, "build-424@shanks") { + t.Fatalf("the note does not say it was abandoned: %s", last) } for _, e := range h.said() { - if e.Event == EventMoved && e.Body["took-over-from"] == "build-424@shanks" && e.Body["stale-claim"] != true { - t.Fatalf("the moved event does not say the claim was stale: %v", e.Body) + if e.Event == EventMoved && e.Body["took-over-from"] == "build-424@shanks" && e.Body["abandoned"] != true { + t.Fatalf("the moved event does not say it was abandoned: %v", e.Body) } } } @@ -258,7 +258,7 @@ func TestAStaleClaimIsFlaggedAndTakenOverWithoutAReason(t *testing.T) { func TestListRefusesAnUnknownFlag(t *testing.T) { h := newHarness(t) _, err := h.call(t, "list", map[string]any{"flag": "stuck"}) - if err == nil || !strings.Contains(err.Error(), "delivered-not-resolved") || !strings.Contains(err.Error(), "stale-claim") { + if err == nil || !strings.Contains(err.Error(), "delivered-not-resolved") || !strings.Contains(err.Error(), "abandoned") { t.Fatalf("want refused naming the flags, got %v", err) } } @@ -308,7 +308,7 @@ func mustView(t *testing.T, h *harness, n uint64) *View { // A pull request closed without merging is withdrawn: mesh-delivery says it `stopped` with no merged_as (its // `closed` event). It is not a fix merged and stuck, not delivered, and not an open pull request: the operator is -// never told it is merged and not delivered, and a worker with nothing else in review holds a stale claim. +// never told it is merged and not delivered, and a worker with nothing else in review holds an abandoned issue. func TestAPullRequestClosedUnmergedIsWithdrawn(t *testing.T) { for _, via := range []string{"transition", "checks"} { t.Run(via, func(t *testing.T) { @@ -347,7 +347,7 @@ func TestAPullRequestClosedUnmergedIsWithdrawn(t *testing.T) { if v["delivered-not-resolved"] == true || countEvents(h, EventReadyToVerify) != 0 { t.Fatalf("a withdrawn pull request reads as delivered: %v", v) } - if v["stale-claim"] != true { + if v["abandoned"] != true { t.Fatalf("a withdrawn pull request reads as open, so the claim is not stale: %v", v) } if d, _ := v["delivery"].(string); !strings.HasPrefix(d, "fix withdrawn: mesh-catalog#9") { diff --git a/modules/mesh-issues/cmd/mesh-issues/delivery.go b/modules/mesh-issues/cmd/mesh-issues/delivery.go index d56c73f0..c1a4240c 100644 --- a/modules/mesh-issues/cmd/mesh-issues/delivery.go +++ b/modules/mesh-issues/cmd/mesh-issues/delivery.go @@ -54,7 +54,7 @@ func reached(state string) bool { func withdrawn(f *Fix) bool { return f != nil && f.State == "stopped" && !f.Merged } // liveFixes is an issue's fixes less those withdrawn: what its delivery, readiness to verify, the stuck sweep and a -// stale claim are measured on. fixPullRequests still names the withdrawn ones, so a pull request reopened is heard. +// abandoned issue are measured on. fixPullRequests still names the withdrawn ones, so a pull request reopened is heard. func liveFixes(is *Issue) []string { var out []string for _, pr := range fixPullRequests(is) { diff --git a/modules/mesh-issues/cmd/mesh-issues/main.go b/modules/mesh-issues/cmd/mesh-issues/main.go index c5e7e8e1..30e1353e 100644 --- a/modules/mesh-issues/cmd/mesh-issues/main.go +++ b/modules/mesh-issues/cmd/mesh-issues/main.go @@ -374,7 +374,7 @@ func Tools(t *Tracker) []stdio.Tool { "delivered. Each row says whether its fix is delivered, and, for an issue at diagnosing or fixing, who " + "works on it (worker) since when, and stale when nobody did anything on it for a day, so it may be " + "taken over. flag delivered-not-resolved keeps the issues in fixing whose every fix (fixed-by and the " + - "pull requests linked) is delivered; flag stale-claim the issues whose worker did nothing on them for a " + + "pull requests linked) is delivered; flag abandoned the issues whose worker did nothing on them for a " + "day with no pull request open, which a move takes over without a reason. Each row says both. Answers " + "what is open in one call.", Input: schema(nil, map[string]any{ @@ -385,7 +385,7 @@ func Tools(t *Tracker) []stdio.Tool { "severity": map[string]any{"type": "string", "enum": Severities, "description": "only issues at this severity"}, "needs": map[string]any{"type": "string", "enum": NeedsKinds, "description": "only issues that need this"}, "ready": map[string]any{"type": "boolean", "description": "only issues ready (true) or waiting (false)"}, - "flag": map[string]any{"type": "string", "enum": Flags, "description": "only issues delivered and not resolved, or the stale claims"}, + "flag": map[string]any{"type": "string", "enum": Flags, "description": "only issues delivered and not resolved, or the abandoned ones"}, }), Run: func(m map[string]any) (any, error) { a, err := readArgs("list", m, "status", "owner", "phrase", "limit", "severity", "needs", "ready", "flag") @@ -433,7 +433,7 @@ func Tools(t *Tracker) []stdio.Tool { "who works on it and its node, as who@node (such as fix-399@g14); the issue shows it until it moves on. " + "Refused while someone else works on it, naming them, unless take-over says why (noted on the issue); a " + "worker who did nothing on it for a day may be taken over from without a reason, and with no pull request open " + - "the answer says the claim was a stale claim. located needs located-in, owners on " + + "the answer says it was abandoned. located needs located-in, owners on " + "the registered list; resolved is reached only from fixing and needs located-in and fixed-by (the pull " + "requests or commits), and an issue in the mesh's core also replay (the replay's id, such as R361) or " + "replay-none (why no replay is possible); wontfix needs a reason. A move to the status it has changes " + diff --git a/modules/mesh-issues/cmd/mesh-issues/ready.go b/modules/mesh-issues/cmd/mesh-issues/ready.go index 2a0ff76b..f2d989b2 100644 --- a/modules/mesh-issues/cmd/mesh-issues/ready.go +++ b/modules/mesh-issues/cmd/mesh-issues/ready.go @@ -291,9 +291,9 @@ type View struct { Delivery string `json:"delivery,omitempty"` // WorkerStale says nothing was done on it by its worker, or anyone, for worker-stale-after (ADR 0284 §4). WorkerStale bool `json:"worker-stale,omitempty"` - // DeliveredNotResolved and StaleClaim are list's flags, said of this issue (novox/hq issue 483). + // DeliveredNotResolved and Abandoned are list's flags, said of this issue (novox/hq issue 483). DeliveredNotResolved bool `json:"delivered-not-resolved,omitempty"` - StaleClaim bool `json:"stale-claim,omitempty"` + Abandoned bool `json:"abandoned,omitempty"` } // View is `get`. @@ -309,7 +309,7 @@ func (t *Tracker) View(n uint64) (*View, error) { now, staleAfter := t.now(), t.workerStaleAfter() return &View{Issue: is, NeededBy: neededBy(all, n), WaitsFor: waitsFor(all, is), Ready: isReady(all, is), Delivery: DeliveryLine(is), WorkerStale: is.Worker != nil && is.Worker.stale(now, staleAfter), - DeliveredNotResolved: deliveredNotResolved(is), StaleClaim: staleClaim(is, now, staleAfter)}, nil + DeliveredNotResolved: deliveredNotResolved(is), Abandoned: abandoned(is, now, staleAfter)}, nil } // letGo says, for an issue that may have just let others go (its fix delivered, or wontfix), every issue it diff --git a/modules/mesh-issues/cmd/mesh-issues/tracker.go b/modules/mesh-issues/cmd/mesh-issues/tracker.go index 4720652c..6aabf517 100644 --- a/modules/mesh-issues/cmd/mesh-issues/tracker.go +++ b/modules/mesh-issues/cmd/mesh-issues/tracker.go @@ -348,9 +348,9 @@ type Moved struct { Since *time.Time `json:"since,omitempty"` // TookOverFrom is the worker a take-over took the issue over from. TookOverFrom string `json:"took-over-from,omitempty"` - // StaleClaim says the worker taken over from had a stale claim: nothing done on it past worker-stale-after and + // Abandoned says the worker taken over from had abandoned it: nothing done on it past worker-stale-after and // no pull request open, so no reason was needed (novox/hq issue 483). - StaleClaim bool `json:"stale-claim,omitempty"` + Abandoned bool `json:"abandoned,omitempty"` // Released is the worker whose work on it the move ended. Released string `json:"released,omitempty"` Said string `json:"said"` @@ -521,9 +521,9 @@ func (t *Tracker) move(by string, a MoveAsk) (*Moved, error) { if takeOver != "" { body["take-over"] = takeOver } - if work.staleClaim { - out.StaleClaim, body["stale-claim"] = true, true - out.Said += fmt.Sprintf(" Taken over from %s, whose claim was a stale claim (nothing done on it for %s and no "+ + if work.abandoned { + out.Abandoned, body["abandoned"] = true, true + out.Said += fmt.Sprintf(" Taken over from %s, who had abandoned it (nothing done on it for %s and no "+ "pull request open); noted on the issue.", work.from, staleAfter) } else { out.Said += fmt.Sprintf(" Taken over from %s; noted on the issue.", work.from) @@ -722,7 +722,7 @@ type ListAsk struct { Needs string // Ready, when set, keeps only the issues ready (true) or not ready (false). Ready *bool - // Flag keeps only the issues delivered and not resolved, or the stale claims (novox/hq issue 483). + // Flag keeps only the issues delivered and not resolved, or the abandoned ones (novox/hq issue 483). Flag string } @@ -744,10 +744,10 @@ type Row struct { Worker string `json:"worker,omitempty"` Since *time.Time `json:"since,omitempty"` Stale bool `json:"stale,omitempty"` - // DeliveredNotResolved says it is in fixing with every fix it names delivered; StaleClaim that its worker + // DeliveredNotResolved says it is in fixing with every fix it names delivered; Abandoned that its worker // did nothing on it for worker-stale-after and no pull request of it is open (novox/hq issue 483). DeliveredNotResolved bool `json:"delivered-not-resolved,omitempty"` - StaleClaim bool `json:"stale-claim,omitempty"` + Abandoned bool `json:"abandoned,omitempty"` } // Listed is the answer to a list. @@ -842,7 +842,7 @@ func (t *Tracker) List(a ListAsk) (*Listed, error) { row.Worker, row.Since, row.Stale = c.By, &since, c.stale(now, staleAfter) } row.DeliveredNotResolved = deliveredNotResolved(is) - row.StaleClaim = staleClaim(is, now, staleAfter) + row.Abandoned = abandoned(is, now, staleAfter) out.Rows = append(out.Rows, row) } } diff --git a/modules/mesh-issues/cmd/mesh-issues/worker.go b/modules/mesh-issues/cmd/mesh-issues/worker.go index 61d58cbb..f7946958 100644 --- a/modules/mesh-issues/cmd/mesh-issues/worker.go +++ b/modules/mesh-issues/cmd/mesh-issues/worker.go @@ -90,9 +90,9 @@ type workOutcome struct { took bool // a worker started, or the issue changed hands from string // the worker it was taken over from wasStale bool // the worker taken over from was stale - // staleClaim is a stale worker with no pull request open: a claim nobody works on (novox/hq issue 483). - staleClaim bool - released string // the worker whose work ended, on a move to a status that has none + // abandoned is a stale worker with no pull request open: the issue is abandoned (novox/hq issue 483). + abandoned bool + released string // the worker whose work ended, on a move to a status that has none } // workerFor applies a move's worker to an issue, under ADR 0284 §3 and §4: another worker not stale is refused @@ -123,7 +123,7 @@ func workerFor(is *Issue, to, by, takeOver string, now time.Time, staleAfter tim return out, nil } out.wasStale = cur.stale(now, staleAfter) - out.staleClaim = out.wasStale && !openPullRequest(is) + out.abandoned = out.wasStale && !openPullRequest(is) if !out.wasStale && takeOver == "" { return out, fmt.Errorf("%s works on issue %d (%s) since %s, last active %s. Ask them, or take it over with "+ "take-over saying why; the reason is noted on the issue. A worker who did nothing on it for %s may be "+ @@ -140,8 +140,8 @@ func takeOverNote(o workOutcome, by, reason string, prev *Worker) string { s := fmt.Sprintf("Taken over by %s from %s, who worked on it since %s and was last active %s", by, o.from, prev.Since.Format(time.RFC3339), prev.Active.Format(time.RFC3339)) switch { - case o.staleClaim: - s += " (a stale claim: nothing done on it past the bound, and no pull request open)" + case o.abandoned: + s += " (stale, and abandoned: no pull request open)" case o.wasStale: s += " (stale)" }