mesh-issues: call an issue its worker left abandoned, not a stale claim (hq ADR 0294)
mesh/merge-gate pass: builds mesh-issues → novox; no bus step; every machine composes with the change as it did without (4 of 4 compose)
mesh/repo-check pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-check-typescript.sh (typescript)
mesh/delivery delivered
mesh/merge-gate pass: builds mesh-issues → novox; no bus step; every machine composes with the change as it did without (4 of 4 compose)
mesh/repo-check pass: its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-check-typescript.sh (typescript)
mesh/delivery delivered
The glossary keeps claim for a module's on a seat.
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`, `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.<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).
|
||||
|
||||
**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
|
||||
|
||||
@@ -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.<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 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
|
||||
}
|
||||
|
||||
@@ -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") {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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 " +
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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)"
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user