mesh-issues: an issue says who works on it, and resolves only from fixing (hq issue 399)
mesh/delivery-group group feat/399-fixing-status checking: 0 of 2 member(s) ready
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 superseded: a newer head of the same pull request
mesh/delivery-group group feat/399-fixing-status checking: 0 of 2 member(s) ready
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 superseded: a newer head of the same pull request
Two agents could start on one issue without seeing each other, and an issue could be resolved without anyone having said they started its fix. A move to diagnosing or fixing now names by, the worker as who@node; get and list show the worker with since and stale; another worker is refused while it is current unless take-over says why (noted); a worker untouched for worker-stale-after (24h) may be taken over from without a reason. Resolved is reached only from fixing. The moved event carries by, since and caller. Fixing counts as not closed for the triage and the duplicate check. hq ADR 0283.
This commit is contained in:
@@ -408,6 +408,7 @@ func TestAnIssueIsOpenedNotedLinkedMovedAndListedOverTheBus(t *testing.T) {
|
||||
t.Fatal("resolved without located-in")
|
||||
}
|
||||
must(i.call("move", map[string]any{"number": 365, "status": "located", "located-in": []string{"mesh-controller internal/gate"}}))
|
||||
must(i.call("move", map[string]any{"number": 365, "status": "fixing", "by": "fix-365@g14"}))
|
||||
if _, err := i.call("move", map[string]any{"number": 365, "status": "resolved", "fixed-by": []string{"mesh-controller#300"}}); err == nil {
|
||||
t.Fatal("a core issue resolved without a replay")
|
||||
}
|
||||
@@ -427,11 +428,16 @@ func TestAnIssueIsOpenedNotedLinkedMovedAndListedOverTheBus(t *testing.T) {
|
||||
var kinds []string
|
||||
for _, e := range events {
|
||||
kinds = append(kinds, e.Key)
|
||||
if e.Body["by"] != Unproven || e.Body["number"] != float64(365) {
|
||||
// The move to fixing carries who works on it; every other change, who acted (ADR 0283 §4).
|
||||
want := Unproven
|
||||
if e.Key == EventMoved && e.Body["to"] == StatusFixing {
|
||||
want = "fix-365@g14"
|
||||
}
|
||||
if e.Body["by"] != want || e.Body["number"] != float64(365) {
|
||||
t.Fatalf("event %+v", e)
|
||||
}
|
||||
}
|
||||
if fmt.Sprint(kinds) != "[opened noted linked moved moved]" {
|
||||
if fmt.Sprint(kinds) != "[opened noted linked moved moved moved]" {
|
||||
t.Fatalf("events %v", kinds)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -124,9 +124,10 @@ type Verdict struct {
|
||||
// closes says whether the verdict closes the newer issue: a certain duplicate, and nothing else.
|
||||
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 or located.
|
||||
// unresolved says whether an issue may be offered, linked or noted by the triage: at open, diagnosing, located or
|
||||
// fixing (ADR 0283 §5).
|
||||
func unresolved(is *Issue) bool {
|
||||
return is.Status == StatusOpen || is.Status == StatusDiagnosing || is.Status == StatusLocated
|
||||
return is.Status == StatusOpen || is.Status == StatusDiagnosing || is.Status == StatusLocated || is.Status == StatusFixing
|
||||
}
|
||||
|
||||
// notePrefix starts every note the triage writes; verdictPrefix the one on the issue it judged.
|
||||
|
||||
@@ -21,17 +21,20 @@ const (
|
||||
IssueCounter = "issue"
|
||||
)
|
||||
|
||||
// The five statuses of an issue, as the development cycle names them (hq 00-META/checks/cycle.py).
|
||||
// The six statuses of an issue, as the development cycle names them (hq playbook 03, ADR 0275, ADR 0283).
|
||||
// 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).
|
||||
const (
|
||||
StatusOpen = "open"
|
||||
StatusDiagnosing = "diagnosing"
|
||||
StatusLocated = "located"
|
||||
StatusFixing = "fixing"
|
||||
StatusResolved = "resolved"
|
||||
StatusWontfix = "wontfix"
|
||||
)
|
||||
|
||||
// Statuses is every status an issue may have, in the order the cycle walks them.
|
||||
var Statuses = []string{StatusOpen, StatusDiagnosing, StatusLocated, StatusResolved, StatusWontfix}
|
||||
var Statuses = []string{StatusOpen, StatusDiagnosing, StatusLocated, StatusFixing, StatusResolved, StatusWontfix}
|
||||
|
||||
// Severities say how much an issue costs (ADR 0279): critical — something the operator relies on is broken, or
|
||||
// data is at risk; major — a rule of the mesh does not hold, or people work around it; minor — a contained fix,
|
||||
@@ -62,7 +65,7 @@ const Unproven = "unproven"
|
||||
type Issue struct {
|
||||
Number uint64 `json:"number"`
|
||||
Title string `json:"title"`
|
||||
// Status is one of the five. It was empty for an issue opened with a folder in hq before the import of
|
||||
// Status is one of the six. It was empty for an issue opened with a folder in hq before the import of
|
||||
// slice 3, whose folder kept its status; the import fills it (to-be 53 §8).
|
||||
Status string `json:"status,omitempty"`
|
||||
Report string `json:"report"`
|
||||
@@ -103,6 +106,8 @@ type Issue struct {
|
||||
ReadyFor []uint64 `json:"ready-for,omitempty"`
|
||||
// Triage is the triage's judgement as it was made, kept before it is applied.
|
||||
Triage *Verdict `json:"triage,omitempty"`
|
||||
// Worker is who works on it, while it is diagnosing or fixing (ADR 0283); nil at every other status.
|
||||
Worker *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"`
|
||||
// Imported says that the issue, or its status, was taken from its folder in hq by the import of
|
||||
|
||||
@@ -44,6 +44,9 @@ 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
|
||||
// duration such as 24h; DefaultWorkerStaleAfter when empty.
|
||||
WorkerStaleAfter string `json:"worker-stale-after,omitempty"`
|
||||
}
|
||||
|
||||
func readSettings(path string) (Settings, error) {
|
||||
@@ -335,7 +338,8 @@ func Tools(t *Tracker) []stdio.Tool {
|
||||
}},
|
||||
{Name: Seat + ".get",
|
||||
Description: "One issue whole, by its number: title, status, severity, what it needs, report, who opened it " +
|
||||
"and when, the owners it is located in, what fixed it and whether that is delivered, its notes and its " +
|
||||
"and when, the owners it is located in, who works on it and since when (worker, and worker-stale when " +
|
||||
"nobody did anything on it for a day), what fixed it and whether that is delivered, its notes and its " +
|
||||
"links; the issues it waits for, those that depend on it (needed-by), and whether it is ready; and " +
|
||||
"whether its fix is delivered, as the delivery last said it (merged, held and why, delivered).",
|
||||
Input: schema([]string{"number"}, map[string]any{"number": number("the issue's number")}),
|
||||
@@ -355,7 +359,9 @@ func Tools(t *Tracker) []stdio.Tool {
|
||||
"those at one status (" + statuses + "), located in one owner, at one severity, needing one thing, " +
|
||||
"ready or not, or whose title or report holds a phrase. An agent looking for work asks severity minor, " +
|
||||
"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. Answers what is open in one call.",
|
||||
"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.",
|
||||
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"),
|
||||
@@ -404,11 +410,15 @@ func Tools(t *Tracker) []stdio.Tool {
|
||||
}},
|
||||
{Name: Seat + ".move",
|
||||
Description: "Change an issue's status — " + statuses + " — with what that status needs, under the " +
|
||||
"development cycle's rules: located needs located-in, owners on the registered list; resolved 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 only what it holds, such as naming the fix while it stays located. " +
|
||||
"A refused move changes nothing and says which rule refused it.",
|
||||
"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 " +
|
||||
"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 " +
|
||||
"only what it holds, such as naming the fix while it stays fixing. A refused move changes nothing and " +
|
||||
"says which rule refused it.",
|
||||
Input: schema([]string{"number", "status"}, map[string]any{
|
||||
"number": number("the issue's number"),
|
||||
"status": map[string]any{"type": "string", "enum": Statuses, "description": "the status it moves to"},
|
||||
@@ -418,9 +428,11 @@ func Tools(t *Tracker) []stdio.Tool {
|
||||
"replay": text("for a core issue resolved: the replay's id in mesh-lab's replays register, such as R361"),
|
||||
"replay-none": text("for a core issue resolved: why no replay is possible"),
|
||||
"reason": text("for wontfix: why it will not be fixed"),
|
||||
"by": text("for diagnosing and fixing: who works on it and its node, as who@node, such as fix-399@g14 or jochen@shanks"),
|
||||
"take-over": text("for diagnosing and fixing, when someone else works on it: why you take it over"),
|
||||
}),
|
||||
Run: func(m map[string]any) (any, error) {
|
||||
a, err := readArgs("move", m, "number", "status", "located-in", "fixed-by", "amended-design", "replay", "replay-none", "reason")
|
||||
a, err := readArgs("move", m, "number", "status", "located-in", "fixed-by", "amended-design", "replay", "replay-none", "reason", "by", "take-over")
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
@@ -431,7 +443,8 @@ func Tools(t *Tracker) []stdio.Tool {
|
||||
for _, f := range []struct {
|
||||
k string
|
||||
p *string
|
||||
}{{"status", &ask.Status}, {"replay", &ask.Replay}, {"replay-none", &ask.ReplayNone}, {"reason", &ask.Reason}} {
|
||||
}{{"status", &ask.Status}, {"replay", &ask.Replay}, {"replay-none", &ask.ReplayNone}, {"reason", &ask.Reason},
|
||||
{"by", &ask.By}, {"take-over", &ask.TakeOver}} {
|
||||
if *f.p, err = a.str(f.k); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
@@ -203,6 +203,14 @@ 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).
|
||||
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 {
|
||||
t.Fatalf("move %d to fixing: %v", n, err)
|
||||
}
|
||||
}
|
||||
|
||||
// call runs one verb as the tool runner does: by its tool, with arguments as JSON gives them.
|
||||
func (h *harness) call(t *testing.T, verb string, args map[string]any) (any, error) {
|
||||
t.Helper()
|
||||
|
||||
@@ -88,6 +88,7 @@ func (t *Tracker) set(by string, a SetAsk, onlyEmpty bool) (*SetDone, error) {
|
||||
if len(changed) == 0 {
|
||||
return errNothing
|
||||
}
|
||||
touch(is, by, t.now())
|
||||
return nil
|
||||
})
|
||||
if errors.Is(err, errNothing) {
|
||||
@@ -270,6 +271,8 @@ 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 bool `json:"worker-stale,omitempty"`
|
||||
}
|
||||
|
||||
// View is `get`.
|
||||
@@ -283,7 +286,7 @@ func (t *Tracker) View(n uint64) (*View, error) {
|
||||
return nil, fmt.Errorf("there is no issue %d: %w", n, ErrNoIssue)
|
||||
}
|
||||
return &View{Issue: is, NeededBy: neededBy(all, n), WaitsFor: waitsFor(all, is), Ready: isReady(all, is),
|
||||
Delivery: DeliveryLine(is)}, nil
|
||||
Delivery: DeliveryLine(is), WorkerStale: is.Worker != nil && is.Worker.stale(t.now(), t.workerStaleAfter())}, nil
|
||||
}
|
||||
|
||||
// letGo says, for an issue that may have just let others go (its fix delivered, or wontfix), every issue it
|
||||
|
||||
@@ -237,7 +237,7 @@ func unquote(v string) string {
|
||||
|
||||
// ParseReport reads one issue report. The folder is its name, `NNN-slug`; the text is the report whole.
|
||||
// What makes a report unreadable is an error: no frontmatter, no title, no status, or a status that
|
||||
// is not one of the five. What is odd but readable is said in Odd.
|
||||
// is not one of the six. What is odd but readable is said in Odd.
|
||||
func ParseReport(folder, text string) (*Report, error) {
|
||||
n, ok := folderNumber(folder)
|
||||
if !ok {
|
||||
|
||||
@@ -313,18 +313,29 @@ 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 string
|
||||
// TakeOver is why the issue is taken over from another worker (ADR 0283 §1).
|
||||
TakeOver string
|
||||
}
|
||||
|
||||
// Moved is the answer to a move.
|
||||
type Moved struct {
|
||||
Number uint64 `json:"number"`
|
||||
From string `json:"from"`
|
||||
To string `json:"to"`
|
||||
Changed []string `json:"changed,omitempty"`
|
||||
By string `json:"by"`
|
||||
Said string `json:"said"`
|
||||
Announced bool `json:"announced"`
|
||||
Warning string `json:"warning,omitempty"`
|
||||
Number uint64 `json:"number"`
|
||||
From string `json:"from"`
|
||||
To string `json:"to"`
|
||||
Changed []string `json:"changed,omitempty"`
|
||||
// By is who works on it, as the move named it; for a move that names nobody, who acted.
|
||||
By string `json:"by"`
|
||||
// Since is when By started on it, for a move to diagnosing or fixing.
|
||||
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"`
|
||||
// Released is the worker whose work on it the move ended.
|
||||
Released string `json:"released,omitempty"`
|
||||
Said string `json:"said"`
|
||||
Announced bool `json:"announced"`
|
||||
Warning string `json:"warning,omitempty"`
|
||||
}
|
||||
|
||||
func known(status string) bool {
|
||||
@@ -391,12 +402,36 @@ 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.
|
||||
var worker, takeOver string
|
||||
if worked(to) {
|
||||
if strings.TrimSpace(a.By) == "" {
|
||||
return nil, fmt.Errorf("a move to %s needs by: who works on it and its node, as who@node, such as "+
|
||||
"fix-399@g14 or jochen@shanks", to)
|
||||
}
|
||||
if worker, err = checkWorker(a.By); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
} else if strings.TrimSpace(a.By) != "" {
|
||||
return nil, fmt.Errorf("by names who works on an issue, which only diagnosing and fixing hold; this moves to %s", to)
|
||||
}
|
||||
if strings.TrimSpace(a.TakeOver) != "" {
|
||||
if !worked(to) {
|
||||
return nil, fmt.Errorf("take-over is for taking over who works on an issue, at diagnosing or fixing; this moves to %s", to)
|
||||
}
|
||||
if takeOver, err = multiLine("take-over", a.TakeOver, MostReason); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
staleAfter := t.workerStaleAfter()
|
||||
|
||||
var from string
|
||||
var changed []string
|
||||
var work workOutcome
|
||||
is, err := t.change(a.Number, func(is *Issue) error {
|
||||
changed = nil
|
||||
from = is.Status
|
||||
at := t.now()
|
||||
// A field counts as changed only when its value does, so a move that changes nothing is refused.
|
||||
set := func(name string, cond bool, apply func()) {
|
||||
if !cond {
|
||||
@@ -414,17 +449,32 @@ func (t *Tracker) move(by string, a MoveAsk) (*Moved, error) {
|
||||
set("replay", replay != "", func() { is.Replay, is.ReplayNone = replay, "" })
|
||||
set("replay-none", replayNone != "", func() { is.ReplayNone, is.Replay = replayNone, "" })
|
||||
set("reason", reason != "", func() { is.Reason = reason })
|
||||
prev := is.Worker
|
||||
if prev != nil {
|
||||
c := *prev
|
||||
prev = &c
|
||||
}
|
||||
h, cerr := workerFor(is, to, worker, takeOver, at, staleAfter)
|
||||
if cerr != nil {
|
||||
return cerr
|
||||
}
|
||||
work = h
|
||||
if work.took {
|
||||
changed = append(changed, "by")
|
||||
}
|
||||
if work.from != "" {
|
||||
is.Notes = append(is.Notes, Note{By: worker, At: at, Text: takeOverNote(work, worker, takeOver, prev)})
|
||||
}
|
||||
// The same status again only to change what it holds: a fix named while the issue stays located, to be
|
||||
// resolved once it is delivered and proven (ADR 0281 §3).
|
||||
if from == to && len(changed) == 0 {
|
||||
return fmt.Errorf("issue %d is already %s", is.Number, to)
|
||||
}
|
||||
if err := cycleRules(is, to); err != nil {
|
||||
if err := cycleRules(is, from, to); err != nil {
|
||||
return err
|
||||
}
|
||||
is.Status = to
|
||||
if (to == StatusResolved || to == StatusWontfix) && from != to {
|
||||
at := t.now()
|
||||
is.Closed = &at
|
||||
} else if to != StatusResolved && to != StatusWontfix {
|
||||
is.Closed = nil
|
||||
@@ -434,9 +484,28 @@ func (t *Tracker) move(by string, a MoveAsk) (*Moved, error) {
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
out := &Moved{Number: is.Number, From: from, To: to, Changed: changed, By: by,
|
||||
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)}
|
||||
body := map[string]any{"number": is.Number, "from": from, "to": to, "by": out.By, "at": t.now()}
|
||||
// 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).
|
||||
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
|
||||
out.By, out.Since = is.Worker.By, &since
|
||||
body["by"], body["since"] = is.Worker.By, since
|
||||
out.Said = fmt.Sprintf("Issue %d moved from %s to %s; %s works on it since %s.", is.Number, from, to,
|
||||
is.Worker.By, since.Format(time.RFC3339))
|
||||
}
|
||||
if work.from != "" {
|
||||
body["took-over-from"] = work.from
|
||||
if takeOver != "" {
|
||||
body["take-over"] = takeOver
|
||||
}
|
||||
out.Said += fmt.Sprintf(" Taken over from %s; noted on the issue.", work.from)
|
||||
}
|
||||
if work.released != "" {
|
||||
body["released"] = work.released
|
||||
}
|
||||
for _, c := range changed {
|
||||
switch c {
|
||||
case "located-in":
|
||||
@@ -457,9 +526,9 @@ func (t *Tracker) move(by string, a MoveAsk) (*Moved, error) {
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// cycleRules is what the development cycle asks of an issue at a status (hq cycle.py, playbook 03, to-be 53
|
||||
// §5), said as the rule that refused it.
|
||||
func cycleRules(is *Issue, to string) 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.
|
||||
func cycleRules(is *Issue, from, to string) error {
|
||||
switch to {
|
||||
case StatusLocated:
|
||||
if len(is.LocatedIn) == 0 {
|
||||
@@ -467,6 +536,12 @@ func cycleRules(is *Issue, 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
|
||||
// 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, "+
|
||||
"who works on it (who@node), when the fix starts, then resolve it", is.Number, from)
|
||||
}
|
||||
if len(is.LocatedIn) == 0 {
|
||||
return fmt.Errorf("issue %d cannot be resolved without located-in: name the owner that was fixed", is.Number)
|
||||
}
|
||||
@@ -508,6 +583,7 @@ func (t *Tracker) note(by string, n uint64, text string) (*Noted, error) {
|
||||
note := Note{By: by, At: t.now(), Text: text}
|
||||
is, err := t.change(n, func(is *Issue) error {
|
||||
is.Notes = append(is.Notes, note)
|
||||
touch(is, by, note.At)
|
||||
return nil
|
||||
})
|
||||
if err != nil {
|
||||
@@ -582,6 +658,7 @@ func (t *Tracker) link(by string, a LinkAsk) (*Linked, error) {
|
||||
return fmt.Errorf("issue %d already has the %s link to %s", is.Number, kind, target)
|
||||
}
|
||||
is.Links = append(is.Links[:i:i], is.Links[i+1:]...)
|
||||
touch(is, by, at)
|
||||
return nil
|
||||
}
|
||||
}
|
||||
@@ -592,6 +669,7 @@ func (t *Tracker) link(by string, a LinkAsk) (*Linked, error) {
|
||||
return fmt.Errorf("issue %d has %d links, the most one issue keeps", is.Number, MostLinks)
|
||||
}
|
||||
is.Links = append(is.Links, Link{Kind: kind, Target: target, By: by, At: at})
|
||||
touch(is, by, at)
|
||||
return nil
|
||||
})
|
||||
if err != nil {
|
||||
@@ -633,6 +711,11 @@ type Row struct {
|
||||
SuggestedOwner string `json:"suggested-owner,omitempty"`
|
||||
// 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).
|
||||
Worker string `json:"worker,omitempty"`
|
||||
Since *time.Time `json:"since,omitempty"`
|
||||
Stale bool `json:"stale,omitempty"`
|
||||
}
|
||||
|
||||
// Listed is the answer to a list.
|
||||
@@ -680,6 +763,7 @@ func (t *Tracker) List(a ListAsk) (*Listed, error) {
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
now, staleAfter := t.now(), t.workerStaleAfter()
|
||||
var numbers []uint64
|
||||
for n := range all {
|
||||
numbers = append(numbers, n)
|
||||
@@ -710,8 +794,13 @@ func (t *Tracker) List(a ListAsk) (*Listed, error) {
|
||||
}
|
||||
out.Matched++
|
||||
if len(out.Rows) < limit {
|
||||
out.Rows = append(out.Rows, Row{Number: n, Status: is.Status, Owner: strings.Join(is.LocatedIn, "; "), Title: is.Title,
|
||||
Severity: is.Severity, Needs: is.Needs, Ready: ready, SuggestedOwner: is.SuggestedOwner, Delivery: DeliveryLine(is)})
|
||||
row := Row{Number: n, Status: is.Status, Owner: strings.Join(is.LocatedIn, "; "), Title: is.Title,
|
||||
Severity: is.Severity, Needs: is.Needs, Ready: ready, SuggestedOwner: is.SuggestedOwner, Delivery: DeliveryLine(is)}
|
||||
if c := is.Worker; c != nil {
|
||||
since := c.Since
|
||||
row.Worker, row.Since, row.Stale = c.By, &since, c.stale(now, staleAfter)
|
||||
}
|
||||
out.Rows = append(out.Rows, row)
|
||||
}
|
||||
}
|
||||
switch {
|
||||
|
||||
@@ -247,7 +247,7 @@ func TestAnIssueWithAFolderIsNumberedTitledAndPointsAtItsFolder(t *testing.T) {
|
||||
t.Fatalf("answer %+v", o)
|
||||
}
|
||||
// Since the import of slice 3, an issue with a folder is whole in the tracker: its status moves here.
|
||||
if _, err := h.Move(MoveAsk{Number: 365, Status: StatusDiagnosing}); err != nil {
|
||||
if _, err := h.Move(MoveAsk{Number: 365, Status: StatusDiagnosing, By: "fix-365@g14"}); err != nil {
|
||||
t.Fatalf("a move of an issue with a folder: %v", err)
|
||||
}
|
||||
if _, err := h.Note(365, "notes are taken"); err != nil {
|
||||
@@ -259,42 +259,75 @@ func TestAnIssueWithAFolderIsNumberedTitledAndPointsAtItsFolder(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// The cycle's rules at the write (to-be 53 §5): one case per row, each refused case changing nothing.
|
||||
// The cycle's rules at the write (to-be 53 §5, ADR 0283): 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
|
||||
cases := []struct {
|
||||
name string
|
||||
located []string // at open
|
||||
from string // the status it is moved to first; "" stays open
|
||||
ask MoveAsk
|
||||
refused string
|
||||
}{
|
||||
{"an unknown status", nil, MoveAsk{Status: "closed"}, "not one of"},
|
||||
{"located without located-in", nil, MoveAsk{Status: StatusLocated}, "without located-in"},
|
||||
{"located with an unregistered owner", nil, MoveAsk{Status: StatusLocated, LocatedIn: []string{"elsewhere"}}, "no registered owner"},
|
||||
{"located with located-in", nil, MoveAsk{Status: StatusLocated, LocatedIn: []string{"mesh-catalog modules/records"}}, ""},
|
||||
{"located, named at open", []string{"hq"}, MoveAsk{Status: StatusLocated}, ""},
|
||||
{"resolved without located-in", nil, MoveAsk{Status: StatusResolved, FixedBy: []string{"hq#1"}}, "without located-in"},
|
||||
{"resolved without fixed-by", []string{"hq"}, MoveAsk{Status: StatusResolved}, "without fixed-by"},
|
||||
{"resolved", []string{"hq"}, MoveAsk{Status: StatusResolved, FixedBy: []string{"hq#280"}}, ""},
|
||||
{"a core issue resolved without a replay", []string{"mesh-controller"}, MoveAsk{Status: StatusResolved, FixedBy: []string{"mesh-controller#9"}}, "core issue"},
|
||||
{"a core issue in the bus resolved without a replay", []string{"mesh-catalog modules/nats"}, MoveAsk{Status: StatusResolved, FixedBy: []string{"mesh-catalog#9"}}, "core issue"},
|
||||
{"a core issue resolved with a replay", []string{"mesh-tools node-tools/internal/bus"}, MoveAsk{Status: StatusResolved, FixedBy: []string{"mesh-tools#25"}, Replay: "R361"}, ""},
|
||||
{"a core issue resolved with replay-none", []string{"mesh-sdk"}, MoveAsk{Status: StatusResolved, FixedBy: []string{"mesh-sdk#16"}, ReplayNone: "it needs two machines"}, ""},
|
||||
{"a replay that names none", []string{"mesh-sdk"}, MoveAsk{Status: StatusResolved, FixedBy: []string{"mesh-sdk#16"}, Replay: "yes"}, "names no replay"},
|
||||
{"replay and replay-none", []string{"mesh-sdk"}, MoveAsk{Status: StatusResolved, FixedBy: []string{"x"}, Replay: "R1", ReplayNone: "y"}, "not both"},
|
||||
{"a replay on another status", []string{"mesh-sdk"}, MoveAsk{Status: StatusLocated, Replay: "R1"}, "what resolving"},
|
||||
{"wontfix without a reason", nil, MoveAsk{Status: StatusWontfix}, "without a reason"},
|
||||
{"wontfix", nil, MoveAsk{Status: StatusWontfix, Reason: "the module is retired"}, ""},
|
||||
{"a reason on another status", nil, MoveAsk{Status: StatusDiagnosing, Reason: "x"}, "what wontfix needs"},
|
||||
{"to the status it has", nil, MoveAsk{Status: StatusOpen}, "already open"},
|
||||
{"diagnosing", nil, MoveAsk{Status: StatusDiagnosing}, ""},
|
||||
{"an amended design that is not one", nil, MoveAsk{Status: StatusDiagnosing, AmendedDesign: []string{"../etc/passwd"}}, "not a design document"},
|
||||
{"a fixed-by of two lines", []string{"hq"}, MoveAsk{Status: StatusResolved, FixedBy: []string{"a\nb"}}, "one line"},
|
||||
{"an unknown status", nil, "", MoveAsk{Status: "closed"}, "not one of"},
|
||||
{"located without located-in", nil, "", MoveAsk{Status: StatusLocated}, "without located-in"},
|
||||
{"located with an unregistered owner", nil, "", MoveAsk{Status: StatusLocated, LocatedIn: []string{"elsewhere"}}, "no registered owner"},
|
||||
{"located with located-in", nil, "", MoveAsk{Status: StatusLocated, LocatedIn: []string{"mesh-catalog modules/records"}}, ""},
|
||||
{"located, named at open", []string{"hq"}, "", MoveAsk{Status: StatusLocated}, ""},
|
||||
{"resolved without located-in", nil, fx, MoveAsk{Status: StatusResolved, FixedBy: []string{"hq#1"}}, "without located-in"},
|
||||
{"resolved without fixed-by", []string{"hq"}, fx, MoveAsk{Status: StatusResolved}, "without fixed-by"},
|
||||
{"resolved", []string{"hq"}, fx, MoveAsk{Status: StatusResolved, FixedBy: []string{"hq#280"}}, ""},
|
||||
{"resolved from open", []string{"hq"}, "", MoveAsk{Status: StatusResolved, FixedBy: []string{"hq#280"}}, "only from fixing"},
|
||||
{"resolved from located", []string{"hq"}, StatusLocated, MoveAsk{Status: StatusResolved, FixedBy: []string{"hq#280"}}, "only from fixing"},
|
||||
{"resolved from diagnosing", []string{"hq"}, StatusDiagnosing, MoveAsk{Status: StatusResolved, FixedBy: []string{"hq#280"}}, "only from fixing"},
|
||||
{"a core issue resolved without a replay", []string{"mesh-controller"}, fx, MoveAsk{Status: StatusResolved, FixedBy: []string{"mesh-controller#9"}}, "core issue"},
|
||||
{"a core issue in the bus resolved without a replay", []string{"mesh-catalog modules/nats"}, fx, MoveAsk{Status: StatusResolved, FixedBy: []string{"mesh-catalog#9"}}, "core issue"},
|
||||
{"a core issue resolved with a replay", []string{"mesh-tools node-tools/internal/bus"}, fx, MoveAsk{Status: StatusResolved, FixedBy: []string{"mesh-tools#25"}, Replay: "R361"}, ""},
|
||||
{"a core issue resolved with replay-none", []string{"mesh-sdk"}, fx, MoveAsk{Status: StatusResolved, FixedBy: []string{"mesh-sdk#16"}, ReplayNone: "it needs two machines"}, ""},
|
||||
{"a replay that names none", []string{"mesh-sdk"}, fx, MoveAsk{Status: StatusResolved, FixedBy: []string{"mesh-sdk#16"}, Replay: "yes"}, "names no replay"},
|
||||
{"replay and replay-none", []string{"mesh-sdk"}, fx, MoveAsk{Status: StatusResolved, FixedBy: []string{"x"}, Replay: "R1", ReplayNone: "y"}, "not both"},
|
||||
{"a replay on another status", []string{"mesh-sdk"}, "", MoveAsk{Status: StatusLocated, Replay: "R1"}, "what resolving"},
|
||||
{"wontfix without a reason", nil, "", MoveAsk{Status: StatusWontfix}, "without a reason"},
|
||||
{"wontfix", nil, "", MoveAsk{Status: StatusWontfix, Reason: "the module is retired"}, ""},
|
||||
{"wontfix from fixing", nil, fx, MoveAsk{Status: StatusWontfix, Reason: "the module is retired"}, ""},
|
||||
{"a reason on another status", nil, "", MoveAsk{Status: StatusDiagnosing, By: "a@g14", Reason: "x"}, "what wontfix needs"},
|
||||
{"to the status it has", nil, "", MoveAsk{Status: StatusOpen}, "already open"},
|
||||
{"diagnosing", nil, "", MoveAsk{Status: StatusDiagnosing, By: "fix-365@g14"}, ""},
|
||||
{"diagnosing without by", nil, "", MoveAsk{Status: StatusDiagnosing}, "needs by"},
|
||||
{"fixing", nil, "", MoveAsk{Status: StatusFixing, By: "Jochen@shanks"}, ""},
|
||||
{"fixing without by", nil, "", MoveAsk{Status: StatusFixing}, "needs by"},
|
||||
{"fixing with a by that names no node", nil, "", MoveAsk{Status: StatusFixing, By: "an agent"}, "who@node"},
|
||||
{"fixing with a by that carries a secret", nil, "", MoveAsk{Status: StatusFixing, By: "sk-ant-api03-abcdefghijklmnopqrstuvwxyz@g14"}, "carries no secret"},
|
||||
{"by on another status", []string{"hq"}, "", MoveAsk{Status: StatusLocated, By: "a@g14"}, "only diagnosing and fixing"},
|
||||
{"take-over on another status", []string{"hq"}, "", MoveAsk{Status: StatusLocated, TakeOver: "x"}, "take-over is for"},
|
||||
{"take-over with nobody to take from", nil, "", MoveAsk{Status: StatusFixing, By: "a@g14", TakeOver: "x"}, "nobody works on"},
|
||||
{"an amended design that is not one", nil, "", MoveAsk{Status: StatusDiagnosing, By: "a@g14", AmendedDesign: []string{"../etc/passwd"}}, "not a design document"},
|
||||
{"a fixed-by of two lines", []string{"hq"}, fx, MoveAsk{Status: StatusResolved, FixedBy: []string{"a\nb"}}, "one line"},
|
||||
}
|
||||
for _, c := range cases {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
h := newHarness(t)
|
||||
n := h.open(t, "an issue", c.located...)
|
||||
from := StatusOpen
|
||||
switch c.from {
|
||||
case "":
|
||||
case StatusFixing:
|
||||
h.fixing(t, n)
|
||||
from = StatusFixing
|
||||
case StatusDiagnosing:
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: c.from, By: "fix@g14"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
from = c.from
|
||||
default:
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: c.from}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
from = c.from
|
||||
}
|
||||
before, _ := h.mem.Get(IssuesState, keyOf(n))
|
||||
events := len(h.said())
|
||||
c.ask.Number = n
|
||||
m, err := h.Move(c.ask)
|
||||
if c.refused != "" {
|
||||
@@ -305,7 +338,7 @@ func TestTheCyclesRulesHoldAtTheWrite(t *testing.T) {
|
||||
if after.Revision != before.Revision || string(after.Value) != string(before.Value) {
|
||||
t.Fatal("a refused move changed the issue")
|
||||
}
|
||||
if len(h.said()) != 1 {
|
||||
if len(h.said()) != events {
|
||||
t.Fatalf("a refused move said %v", h.said())
|
||||
}
|
||||
return
|
||||
@@ -314,16 +347,25 @@ func TestTheCyclesRulesHoldAtTheWrite(t *testing.T) {
|
||||
t.Fatalf("refused: %v", err)
|
||||
}
|
||||
is, _ := h.Get(n)
|
||||
if is.Status != c.ask.Status || m.From != StatusOpen || m.To != c.ask.Status {
|
||||
if is.Status != c.ask.Status || m.From != from || m.To != c.ask.Status {
|
||||
t.Fatalf("moved %+v, kept %+v", m, is)
|
||||
}
|
||||
closed := c.ask.Status == StatusResolved || c.ask.Status == StatusWontfix
|
||||
if (is.Closed != nil) != closed {
|
||||
t.Fatalf("closed %v at %s", is.Closed, is.Status)
|
||||
}
|
||||
wantBy := Unproven
|
||||
if worked(c.ask.Status) {
|
||||
wantBy = strings.ToLower(c.ask.By)
|
||||
if is.Worker == nil || is.Worker.By != wantBy || is.Worker.Since.IsZero() {
|
||||
t.Fatalf("worker %+v", is.Worker)
|
||||
}
|
||||
} else if is.Worker != nil {
|
||||
t.Fatalf("a worker kept at %s: %+v", is.Status, is.Worker)
|
||||
}
|
||||
ev := h.said()
|
||||
if last := ev[len(ev)-1]; last.Event != EventMoved || last.Body["to"] != c.ask.Status || last.Body["from"] != StatusOpen ||
|
||||
last.Body["by"] != Unproven {
|
||||
if last := ev[len(ev)-1]; last.Event != EventMoved || last.Body["to"] != c.ask.Status || last.Body["from"] != from ||
|
||||
last.Body["by"] != wantBy || last.Body["caller"] != Unproven {
|
||||
t.Fatalf("event %+v", last)
|
||||
}
|
||||
})
|
||||
@@ -333,6 +375,7 @@ func TestTheCyclesRulesHoldAtTheWrite(t *testing.T) {
|
||||
func TestReopeningClearsWhenItClosed(t *testing.T) {
|
||||
h := newHarness(t)
|
||||
n := h.open(t, "x", "hq")
|
||||
h.fixing(t, n)
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: StatusResolved, FixedBy: []string{"hq#1"}}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
@@ -347,7 +390,7 @@ func TestReopeningClearsWhenItClosed(t *testing.T) {
|
||||
|
||||
func TestAMoveOfAnIssueThatDoesNotExistIsRefused(t *testing.T) {
|
||||
h := newHarness(t)
|
||||
if _, err := h.Move(MoveAsk{Number: 9, Status: StatusDiagnosing}); !errors.Is(err, ErrNoIssue) {
|
||||
if _, err := h.Move(MoveAsk{Number: 9, Status: StatusDiagnosing, By: "a@g14"}); !errors.Is(err, ErrNoIssue) {
|
||||
t.Fatalf("%v", err)
|
||||
}
|
||||
if _, err := h.Get(9); err == nil || !strings.Contains(err.Error(), "there is no issue 9") {
|
||||
@@ -483,7 +526,7 @@ func TestListIsNewestFirstAndNarrows(t *testing.T) {
|
||||
h.open(t, "the bus drops a message", "mesh-catalog modules/nats")
|
||||
h.open(t, "a check passes and did nothing", "mesh-controller internal/catalogue")
|
||||
n := h.open(t, "the console names a ghost")
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: StatusDiagnosing}); err != nil {
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: StatusDiagnosing, By: "a@g14"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
l, err := h.List(ListAsk{})
|
||||
|
||||
@@ -77,6 +77,7 @@ func resolve(t *testing.T, h *harness, n uint64, fixedBy ...string) {
|
||||
if len(fixedBy) == 0 {
|
||||
fixedBy = []string{"mesh-catalog#1"}
|
||||
}
|
||||
h.fixing(t, n)
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: StatusResolved, LocatedIn: []string{"mesh-catalog modules/records"},
|
||||
FixedBy: fixedBy}); err != nil {
|
||||
t.Fatal(err)
|
||||
|
||||
@@ -0,0 +1,146 @@
|
||||
package main
|
||||
|
||||
// Who works on an issue (novox/hq ADR 0283). 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
|
||||
// from a caller for `worker-stale-after` (24 hours) is stale: `list` says so, and it may be taken over without a
|
||||
// reason.
|
||||
//
|
||||
// `by` is what the worker says it is, not who called: the runtime does not yet hand a module its caller (hq
|
||||
// issue 340), so who acted stays recorded as the runtime proves it (Unproven), and the worker is a way for
|
||||
// agents and people to see each other, not a lock against someone giving a false name.
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"regexp"
|
||||
"strings"
|
||||
"time"
|
||||
)
|
||||
|
||||
// Worker is who works on an issue, since when, and when a caller last did anything on it.
|
||||
type Worker struct {
|
||||
By string `json:"by"`
|
||||
Since time.Time `json:"since"`
|
||||
// Active is the last move, note, link or set on the issue by a caller, not by the tracker itself.
|
||||
Active time.Time `json:"active"`
|
||||
}
|
||||
|
||||
// DefaultWorkerStaleAfter is how long a worker stays current with nothing done on its issue: a working day and a
|
||||
// night, so an agent that stopped (its session ended, its machine asleep) frees the issue by the next day, and
|
||||
// one that works on it keeps it by noting what it found.
|
||||
const DefaultWorkerStaleAfter = 24 * time.Hour
|
||||
|
||||
// MostWorker is the most a worker's name, who@node, may be.
|
||||
const MostWorker = 120
|
||||
|
||||
// workerName is who@node: who is the agent or person (letters, digits and . _ / -, such as fix-399 or jochen),
|
||||
// node the machine it works on.
|
||||
var workerName = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._/-]*@[a-z0-9][a-z0-9-]*$`)
|
||||
|
||||
// worked says whether an issue at a status has a worker: at diagnosing and fixing.
|
||||
func worked(status string) bool { return status == StatusDiagnosing || status == StatusFixing }
|
||||
|
||||
// checkWorker is `by` as kept: who@node, in lower case.
|
||||
func checkWorker(s string) (string, error) {
|
||||
s, err := singleLine("by", s, MostWorker)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
s = strings.ToLower(s)
|
||||
if !workerName.MatchString(s) {
|
||||
return "", fmt.Errorf("by %q is not who works on it and its node: give who@node, the agent or person and the "+
|
||||
"machine it works on, such as fix-399@g14 or jochen@shanks", s)
|
||||
}
|
||||
return s, nil
|
||||
}
|
||||
|
||||
// ownActor says whether a write is the tracker's own (the triage, the duplicate check, the delivery's notes, the
|
||||
// import), which keeps no worker current.
|
||||
func ownActor(by string) bool { return by == GroomBy || by == LegacyBy || by == ImportedBy }
|
||||
|
||||
// touch marks a caller's write on an issue as activity of its worker.
|
||||
func touch(is *Issue, by string, at time.Time) {
|
||||
if is.Worker != nil && !ownActor(by) {
|
||||
is.Worker.Active = at
|
||||
}
|
||||
}
|
||||
|
||||
// stale says whether nothing was done on the issue past the bound since the worker last was.
|
||||
func (w *Worker) stale(now time.Time, after time.Duration) bool {
|
||||
last := w.Active
|
||||
if last.Before(w.Since) {
|
||||
last = w.Since
|
||||
}
|
||||
return now.Sub(last) > after
|
||||
}
|
||||
|
||||
// workerStaleAfter is the setting `worker-stale-after`, or DefaultWorkerStaleAfter when it is empty or unreadable.
|
||||
func (t *Tracker) workerStaleAfter() time.Duration {
|
||||
if s, err := t.Settings(); err == nil && strings.TrimSpace(s.WorkerStaleAfter) != "" {
|
||||
if d, err := time.ParseDuration(strings.TrimSpace(s.WorkerStaleAfter)); err == nil && d > 0 {
|
||||
return d
|
||||
}
|
||||
}
|
||||
return DefaultWorkerStaleAfter
|
||||
}
|
||||
|
||||
// workOutcome is what a move did to an issue's worker.
|
||||
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
|
||||
}
|
||||
|
||||
// workerFor applies a move's worker to an issue, under ADR 0283 §1 and §2: 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
|
||||
cur := is.Worker
|
||||
if !worked(to) {
|
||||
if cur != nil {
|
||||
out.released = cur.By
|
||||
is.Worker = nil
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
if cur != nil && cur.By == by {
|
||||
if takeOver != "" {
|
||||
return out, fmt.Errorf("%s works on issue %d already: take-over is for taking it over from someone else", by, is.Number)
|
||||
}
|
||||
cur.Active = now
|
||||
return out, nil
|
||||
}
|
||||
if cur == nil {
|
||||
if takeOver != "" {
|
||||
return out, fmt.Errorf("nobody works on issue %d: take-over is for taking it over from someone else; move it without one", is.Number)
|
||||
}
|
||||
is.Worker = &Worker{By: by, Since: now, Active: now}
|
||||
out.took = true
|
||||
return out, nil
|
||||
}
|
||||
out.wasStale = cur.stale(now, staleAfter)
|
||||
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 "+
|
||||
"taken over from without a reason", cur.By, is.Number, is.Status, cur.Since.Format(time.RFC3339),
|
||||
cur.Active.Format(time.RFC3339), staleAfter)
|
||||
}
|
||||
out.from, out.took = cur.By, true
|
||||
is.Worker = &Worker{By: by, Since: now, Active: now}
|
||||
return out, nil
|
||||
}
|
||||
|
||||
// takeOverNote is the note a take-over leaves on the issue.
|
||||
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 {
|
||||
s += " (stale)"
|
||||
}
|
||||
if reason != "" {
|
||||
s += ". Why: " + reason
|
||||
}
|
||||
return s + ". (novox/hq ADR 0283)"
|
||||
}
|
||||
@@ -0,0 +1,246 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
// clocked gives a harness a clock the test moves.
|
||||
func clocked(h *harness) *time.Time {
|
||||
now := time.Date(2026, 10, 10, 12, 0, 0, 0, time.UTC)
|
||||
h.Tracker.Now = func() time.Time { return now }
|
||||
return &now
|
||||
}
|
||||
|
||||
// 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).
|
||||
func TestAnotherWorkerIsRefusedNamingTheWorker(t *testing.T) {
|
||||
for _, to := range []string{StatusDiagnosing, StatusFixing} {
|
||||
t.Run(to, func(t *testing.T) {
|
||||
h := newHarness(t)
|
||||
now := clocked(h)
|
||||
n := h.open(t, "the counter skips a number", "hq")
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: StatusDiagnosing, By: "fix-a@g14"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
*now = now.Add(23 * time.Hour)
|
||||
before, _ := h.mem.Get(IssuesState, keyOf(n))
|
||||
events := len(h.said())
|
||||
_, err := h.Move(MoveAsk{Number: n, Status: to, By: "fix-b@ace"})
|
||||
if err == nil || !strings.Contains(err.Error(), "fix-a@g14") || !strings.Contains(err.Error(), "take-over") {
|
||||
t.Fatalf("want refused naming the worker, got %v", err)
|
||||
}
|
||||
after, _ := h.mem.Get(IssuesState, keyOf(n))
|
||||
if after.Revision != before.Revision || len(h.said()) != events {
|
||||
t.Fatal("a refused move changed the issue or said an event")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// The worker moves on from diagnosing to fixing as the same worker; its since stands.
|
||||
func TestTheWorkerMovesOn(t *testing.T) {
|
||||
h := newHarness(t)
|
||||
now := clocked(h)
|
||||
n := h.open(t, "x", "hq")
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: StatusDiagnosing, By: "fix-a@g14"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
since := *now
|
||||
*now = now.Add(time.Hour)
|
||||
m, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "FIX-A@g14"})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
is, _ := h.Get(n)
|
||||
if is.Worker.By != "fix-a@g14" || !is.Worker.Since.Equal(since) || !is.Worker.Active.Equal(*now) || m.By != "fix-a@g14" ||
|
||||
m.Since == nil || !m.Since.Equal(since) || m.TookOverFrom != "" {
|
||||
t.Fatalf("worker %+v, answer %+v", is.Worker, m)
|
||||
}
|
||||
// Naming the fix while it stays fixing is the worker's too.
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, FixedBy: []string{"hq#1"}}); err == nil ||
|
||||
!strings.Contains(err.Error(), "needs by") {
|
||||
t.Fatalf("a move to fixing without by: %v", err)
|
||||
}
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-b@ace", FixedBy: []string{"hq#1"}}); err == nil {
|
||||
t.Fatal("another named the fix while the worker was current")
|
||||
}
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-a@g14", FixedBy: []string{"hq#1"}}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-a@g14", TakeOver: "mine"}); err == nil ||
|
||||
!strings.Contains(err.Error(), "already") {
|
||||
t.Fatalf("a take-over of one's own work: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// A take-over gives a reason, the reason is noted, and the moved event says from whom (ADR 0283 §1, §4).
|
||||
func TestATakeOverNeedsAReasonAndIsNoted(t *testing.T) {
|
||||
h := newHarness(t)
|
||||
clocked(h)
|
||||
n := h.open(t, "x", "hq")
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-a@g14"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
m, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "jochen@shanks", TakeOver: "the operator finishes it by hand"})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
is, _ := h.Get(n)
|
||||
if is.Worker.By != "jochen@shanks" || m.TookOverFrom != "fix-a@g14" || len(is.Notes) != 1 ||
|
||||
!strings.Contains(is.Notes[0].Text, "from fix-a@g14") || !strings.Contains(is.Notes[0].Text, "the operator finishes it by hand") ||
|
||||
is.Notes[0].By != "jochen@shanks" {
|
||||
t.Fatalf("worker %+v, notes %+v, answer %+v", is.Worker, is.Notes, m)
|
||||
}
|
||||
ev := h.said()
|
||||
last := ev[len(ev)-1]
|
||||
if last.Event != EventMoved || last.Body["by"] != "jochen@shanks" || last.Body["took-over-from"] != "fix-a@g14" ||
|
||||
last.Body["take-over"] != "the operator finishes it by hand" || last.Body["caller"] != Unproven {
|
||||
t.Fatalf("event %+v", last)
|
||||
}
|
||||
}
|
||||
|
||||
// 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.
|
||||
func TestAStaleWorkerIsShownAndMayBeTakenOverWithoutAReason(t *testing.T) {
|
||||
h := newHarness(t)
|
||||
now := clocked(h)
|
||||
n := h.open(t, "x", "hq")
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-a@g14"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
since := *now
|
||||
*now = now.Add(20 * time.Hour)
|
||||
if _, err := h.Note(n, "still on it: the counter's revision is read twice"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
*now = now.Add(10 * time.Hour) // 30 h after the start, 10 h after the worker's note
|
||||
if _, err := h.note(GroomBy, n, "Triage: a note of the tracker's own"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
l, _ := h.List(ListAsk{})
|
||||
if r := l.Rows[0]; r.Worker != "fix-a@g14" || r.Since == nil || !r.Since.Equal(since) || r.Stale {
|
||||
t.Fatalf("row %+v: the worker's note keeps the worker current", r)
|
||||
}
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-b@ace"}); err == nil {
|
||||
t.Fatal("a current worker was taken over without a reason")
|
||||
}
|
||||
*now = now.Add(15 * time.Hour) // 25 h after the worker's note; the tracker's note 15 h ago does not count
|
||||
l, _ = h.List(ListAsk{})
|
||||
if r := l.Rows[0]; !r.Stale || r.Worker != "fix-a@g14" {
|
||||
t.Fatalf("row %+v: want stale", r)
|
||||
}
|
||||
v, _ := h.View(n)
|
||||
if !v.WorkerStale {
|
||||
t.Fatalf("get %+v: want worker-stale", v)
|
||||
}
|
||||
m, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-b@ace"})
|
||||
if err != nil {
|
||||
t.Fatalf("a stale worker taken over from without a reason: %v", err)
|
||||
}
|
||||
is, _ := h.Get(n)
|
||||
last := is.Notes[len(is.Notes)-1]
|
||||
if m.TookOverFrom != "fix-a@g14" || is.Worker.By != "fix-b@ace" || !strings.Contains(last.Text, "stale") {
|
||||
t.Fatalf("answer %+v, notes %+v", m, is.Notes)
|
||||
}
|
||||
l, _ = h.List(ListAsk{})
|
||||
if r := l.Rows[0]; r.Stale || r.Worker != "fix-b@ace" {
|
||||
t.Fatalf("row %+v after the take-over", r)
|
||||
}
|
||||
}
|
||||
|
||||
// The bound is the setting worker-stale-after.
|
||||
func TestTheStaleBoundIsTheSetting(t *testing.T) {
|
||||
h := newHarness(t)
|
||||
now := clocked(h)
|
||||
h.settings.WorkerStaleAfter = "1h"
|
||||
n := h.open(t, "x", "hq")
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: StatusDiagnosing, By: "fix-a@g14"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
*now = now.Add(59 * time.Minute)
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: StatusDiagnosing, By: "fix-b@ace"}); err == nil {
|
||||
t.Fatal("taken under the bound")
|
||||
}
|
||||
*now = now.Add(2 * time.Minute)
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: StatusDiagnosing, By: "fix-b@ace"}); err != nil {
|
||||
t.Fatalf("past the bound: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// A move to a status that has no worker ends the work, and the event says whose ended.
|
||||
func TestAMoveOnEndsTheWork(t *testing.T) {
|
||||
h := newHarness(t)
|
||||
clocked(h)
|
||||
n := h.open(t, "x", "hq")
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: StatusDiagnosing, By: "fix-a@g14"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
m, err := h.Move(MoveAsk{Number: n, Status: StatusLocated})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
is, _ := h.Get(n)
|
||||
ev := h.said()
|
||||
if is.Worker != nil || m.Released != "fix-a@g14" || ev[len(ev)-1].Body["released"] != "fix-a@g14" || m.By != Unproven {
|
||||
t.Fatalf("worker %+v, answer %+v, event %+v", is.Worker, m, ev[len(ev)-1])
|
||||
}
|
||||
// Anyone may start on it now, without a take-over.
|
||||
if _, err := h.Move(MoveAsk{Number: n, Status: StatusFixing, By: "fix-b@ace"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
|
||||
// The duplicate check and the triage treat fixing as not closed, like diagnosing and located (ADR 0283 §5).
|
||||
func TestFixingIsNotClosed(t *testing.T) {
|
||||
for _, s := range Statuses {
|
||||
want := s != StatusResolved && s != StatusWontfix
|
||||
if got := unresolved(&Issue{Status: s}); got != want {
|
||||
t.Errorf("unresolved at %s: %v, want %v", s, got, want)
|
||||
}
|
||||
}
|
||||
h := newHarness(t)
|
||||
n := h.open(t, "x", "hq")
|
||||
h.fixing(t, n)
|
||||
l, _ := h.List(ListAsk{Status: StatusFixing})
|
||||
if len(l.Rows) != 1 || l.Rows[0].Number != n || l.Rows[0].Worker != "fix@g14" {
|
||||
t.Fatalf("%+v", l)
|
||||
}
|
||||
}
|
||||
|
||||
// Over the verb as the tool runner calls it: by and take-over are arguments, and fixing is a status the schema
|
||||
// offers.
|
||||
func TestTheMoveVerbTakesByAndTakeOver(t *testing.T) {
|
||||
h := newHarness(t)
|
||||
clocked(h)
|
||||
n := h.open(t, "x", "hq")
|
||||
if _, err := h.call(t, "move", map[string]any{"number": n, "status": "fixing", "by": "fix-a@g14"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, err := h.call(t, "move", map[string]any{"number": n, "status": "fixing", "by": "fix-b@ace"}); err == nil {
|
||||
t.Fatal("taken without a reason over the verb")
|
||||
}
|
||||
if _, err := h.call(t, "move", map[string]any{"number": n, "status": "fixing", "by": "fix-b@ace", "take-over": "fix-a's session ended"}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
got, err := h.call(t, "get", map[string]any{"number": n})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
raw, _ := json.Marshal(got)
|
||||
if !strings.Contains(string(raw), `"worker":{"by":"fix-b@ace"`) {
|
||||
t.Fatalf("get %s", raw)
|
||||
}
|
||||
for _, tool := range Tools(h.Tracker) {
|
||||
if tool.Name != Seat+".move" {
|
||||
continue
|
||||
}
|
||||
raw, _ := json.Marshal(tool.Input)
|
||||
if !strings.Contains(string(raw), `"fixing"`) || !strings.Contains(string(raw), `"take-over"`) ||
|
||||
!strings.Contains(tool.Description, "only from fixing") {
|
||||
t.Fatalf("move's schema %s", raw)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user