Merge pull request 'mesh-issues: say a delivered fix left in fixing, and flag an abandoned issue (hq issue 483, ADR 0294)' (#212) from fix/483-a-delivered-fix-is-said into main
This commit was merged in pull request #212.
This commit is contained in:
@@ -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 `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 |
|
||||
@@ -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
|
||||
@@ -110,6 +110,30 @@ 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 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
|
||||
`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.<number>`, "issue N: fixed and delivered, not resolved; <worker> works on it",
|
||||
cleared there when the issue leaves fixing or names a fix not delivered yet (at its `moved`, or the next sweep).
|
||||
|
||||
**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 `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
|
||||
`issue-tracker.fix-stuck.<number>`), cleared there when every fix of the issue is delivered. A module that runs only
|
||||
@@ -244,7 +268,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).
|
||||
|
||||
```
|
||||
{
|
||||
@@ -260,7 +286,7 @@ as the credential, which ADR 0266 keeps agents from reading (hq issue 333).
|
||||
- `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/<module>`. An entry of `located-in` is an owner, or an owner followed by a space or a
|
||||
|
||||
@@ -519,7 +519,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"
|
||||
|
||||
@@ -0,0 +1,190 @@
|
||||
package main
|
||||
|
||||
// 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:
|
||||
//
|
||||
// - 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.<number>`, 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 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).
|
||||
|
||||
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"
|
||||
FlagAbandoned = "abandoned"
|
||||
)
|
||||
|
||||
// Flags is every flag list takes.
|
||||
var Flags = []string{FlagDeliveredNotResolved, FlagAbandoned}
|
||||
|
||||
// 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 liveFixes(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(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),
|
||||
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, d.Merged, 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. One closed without merging
|
||||
// is not open.
|
||||
func openPullRequest(is *Issue) bool {
|
||||
for _, pr := range liveFixes(is) {
|
||||
f := fixOf(is, pr)
|
||||
if f == nil || (!f.Merged && !contains(is.Delivered, pr)) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// 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 abandoned(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 FlagAbandoned:
|
||||
return abandoned(is, now, staleAfter)
|
||||
}
|
||||
return false
|
||||
}
|
||||
@@ -0,0 +1,407 @@
|
||||
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 <repository>#<number>.
|
||||
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 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")
|
||||
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": "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": "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["abandoned"] != true {
|
||||
t.Fatalf("get does not say it: %v", v)
|
||||
}
|
||||
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("an abandoned issue was not taken over without a reason: %v", err)
|
||||
}
|
||||
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, "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["abandoned"] != true {
|
||||
t.Fatalf("the moved event does not say it was abandoned: %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(), "abandoned") {
|
||||
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
|
||||
}
|
||||
|
||||
// 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 an abandoned issue.
|
||||
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["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") {
|
||||
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) }
|
||||
@@ -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
|
||||
// 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) {
|
||||
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 := pullRequests(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 := pullRequests(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
|
||||
@@ -128,7 +154,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 +203,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 := 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
|
||||
// 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 +253,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 +271,7 @@ func stillStuck(is *Issue) bool {
|
||||
if is.Status == StatusWontfix {
|
||||
return false
|
||||
}
|
||||
named := pullRequests(is)
|
||||
named := liveFixes(is)
|
||||
for _, f := range is.Fixes {
|
||||
if f.StuckSaid && contains(named, f.PullRequest) && !contains(is.Delivered, f.PullRequest) {
|
||||
return true
|
||||
@@ -268,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
|
||||
@@ -287,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.
|
||||
@@ -305,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
|
||||
}
|
||||
}
|
||||
|
||||
@@ -334,7 +393,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
|
||||
}
|
||||
@@ -358,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
|
||||
}
|
||||
}
|
||||
@@ -395,8 +454,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 +468,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 +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 := pullRequests(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 {
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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"
|
||||
@@ -102,12 +102,15 @@ 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"`
|
||||
// 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"`
|
||||
|
||||
@@ -44,9 +44,13 @@ 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
|
||||
// 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 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{
|
||||
"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 abandoned ones"},
|
||||
}),
|
||||
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 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 " +
|
||||
@@ -614,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{
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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 `<repository>#<number>`, 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 {
|
||||
@@ -271,8 +289,11 @@ 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 Abandoned are list's flags, said of this issue (novox/hq issue 483).
|
||||
DeliveredNotResolved bool `json:"delivered-not-resolved,omitempty"`
|
||||
Abandoned bool `json:"abandoned,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), 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
|
||||
|
||||
@@ -333,9 +333,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
|
||||
}
|
||||
|
||||
@@ -351,6 +351,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"`
|
||||
// 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).
|
||||
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"`
|
||||
@@ -422,7 +425,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 +510,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
|
||||
@@ -521,7 +524,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.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)
|
||||
}
|
||||
}
|
||||
if work.released != "" {
|
||||
body["released"] = work.released
|
||||
@@ -547,7 +556,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:
|
||||
@@ -556,7 +565,7 @@ func cycleRules(is *Issue, from, to string) error {
|
||||
"(a repository, or \"mesh-catalog modules/<module>\")", 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, "+
|
||||
@@ -716,6 +725,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 abandoned ones (novox/hq issue 483).
|
||||
Flag string
|
||||
}
|
||||
|
||||
// Row is one issue in a list: short enough to read in a chat.
|
||||
@@ -732,10 +743,14 @@ 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"`
|
||||
// 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"`
|
||||
Abandoned bool `json:"abandoned,omitempty"`
|
||||
}
|
||||
|
||||
// Listed is the answer to a list.
|
||||
@@ -774,6 +789,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
|
||||
@@ -802,6 +823,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
|
||||
@@ -820,6 +844,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.Abandoned = abandoned(is, now, staleAfter)
|
||||
out.Rows = append(out.Rows, row)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 := pullRequests(is)
|
||||
prs := liveFixes(is)
|
||||
if len(prs) == 0 {
|
||||
if len(withdrawnFixes(is)) > 0 {
|
||||
return "withdrawn"
|
||||
}
|
||||
return ""
|
||||
}
|
||||
var failed, held, merged, open, unheard bool
|
||||
|
||||
@@ -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
|
||||
@@ -90,10 +90,12 @@ 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
|
||||
// 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 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
|
||||
@@ -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.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 "+
|
||||
@@ -136,11 +139,14 @@ 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.abandoned:
|
||||
s += " (stale, and abandoned: no pull request open)"
|
||||
case o.wasStale:
|
||||
s += " (stale)"
|
||||
}
|
||||
if reason != "" {
|
||||
s += ". Why: " + reason
|
||||
}
|
||||
return s + ". (novox/hq ADR 0283)"
|
||||
return s + ". (novox/hq ADR 0284)"
|
||||
}
|
||||
|
||||
@@ -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) {
|
||||
|
||||
File diff suppressed because one or more lines are too long
Reference in New Issue
Block a user