From 2ab74991b856e5a02f3db101eaf94dd94d2656c1 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 13 Nov 2026 00:48:36 +0100 Subject: [PATCH] checker_ask_again: an unread backlog and a recheck ask refuse too (issue 1373, review) Principle 3: unread is never empty. build_usage's notes refuse; a waiting or held recheck ask refuses like a check ask. Claude-Session: https://claude.ai/code/session_01NcbgfGKaxw984XyAPfLm4h --- .../cmd/mesh-delivery-checker-tools/main.go | 5 +++++ .../cmd/mesh-delivery-checker-tools/main_test.go | 14 ++++++++++++++ .../internal/check/askagain.go | 4 ++-- .../internal/check/lostask_test.go | 2 ++ 4 files changed, 23 insertions(+), 2 deletions(-) diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go index 4ef85053..e46321f8 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main.go @@ -7,6 +7,7 @@ import ( "encoding/json" "fmt" "os" + "strings" "sync/atomic" "time" @@ -515,10 +516,14 @@ func backlogOf(ask func(string, any) (json.RawMessage, error)) func() ([]check.B } var a struct { Backlog []check.BacklogRow `json:"backlog"` + Notes []string `json:"notes"` } if err := json.Unmarshal(answer, &a); err != nil { return nil, fmt.Errorf("%s answered what does not decode: %w", buildUsageTool, err) } + if len(a.Notes) > 0 { // the backlog was not read: unread is never empty + return nil, fmt.Errorf("%s did not read the backlog: %s", buildUsageTool, strings.Join(a.Notes, "; ")) + } return a.Backlog, nil } } diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main_test.go index a765459f..49f907bf 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker-tools/main_test.go @@ -165,6 +165,20 @@ func TestTheToolsServedAreTheFiveReadTools(t *testing.T) { } }) } + // 1373: the build seats' backlog is read through build_usage; one it says it did not read is never taken for empty. + for _, c := range []struct { + name, answer string + rows int + failed bool + }{ + {name: "a backlog read", answer: `{"backlog":[{"language":"go","kind":"check","waiting":2}]}`, rows: 1}, + {name: "a backlog with a note says it was not read", answer: `{"backlog":[],"notes":["the seats' streams cannot be read here"]}`, failed: true}, + } { + rows, err := backlogOf(func(string, any) (json.RawMessage, error) { return json.RawMessage(c.answer), nil })() + if (err != nil) != c.failed || len(rows) != c.rows || (c.failed && !strings.Contains(err.Error(), "cannot be read here")) { + t.Errorf("1373 backlog, %s: %v, %v", c.name, rows, err) + } + } // With nothing to read from a tool says so and changes nothing. for _, tl := range Tools(check.Reader{}) { if tl.Name == check.StatusReadTool { diff --git a/modules/mesh-delivery-checker/internal/check/askagain.go b/modules/mesh-delivery-checker/internal/check/askagain.go index 3a3b0697..4cc004ee 100644 --- a/modules/mesh-delivery-checker/internal/check/askagain.go +++ b/modules/mesh-delivery-checker/internal/check/askagain.go @@ -255,7 +255,7 @@ type BacklogRow struct { Error string `json:"error,omitempty"` } -// noCheckAskWaits is nil when no check ask waits or is held on any build seat's stream and every row was read; else it +// noCheckAskWaits is nil when no check or recheck ask waits or is held on any build seat's stream and every row was read; else it // names the stream that holds asks. It cannot tell whose ask one is, so any check ask refuses: asking again could run it twice. func (r Reader) noCheckAskWaits() error { rows, err := r.Backlog() @@ -263,7 +263,7 @@ func (r Reader) noCheckAskWaits() error { return fmt.Errorf("the build seats' streams cannot be read (%v), so whether an ask of the check waits in one cannot be told; asking again could run it twice", err) } for _, row := range rows { - if row.Kind != "check" { + if row.Kind != "check" && row.Kind != "recheck" { // both kinds are the checker's asks (judge.go) continue } if row.Error != "" { diff --git a/modules/mesh-delivery-checker/internal/check/lostask_test.go b/modules/mesh-delivery-checker/internal/check/lostask_test.go index 5cc7dbc5..5712a732 100644 --- a/modules/mesh-delivery-checker/internal/check/lostask_test.go +++ b/modules/mesh-delivery-checker/internal/check/lostask_test.go @@ -340,6 +340,8 @@ func TestACheckWhoseAskJudgedNothingIsAskedAgainOrRead(t *testing.T) { bad string }{{"a check ask waits", []BacklogRow{{Language: "go", Kind: "check", Waiting: 1}}, "holds asks"}, {"a check ask is held", []BacklogRow{{Language: "go", Kind: "check", Held: 1}}, "holds asks"}, + {"a recheck ask waits", []BacklogRow{{Language: "go", Kind: "recheck", Waiting: 1}}, "holds asks"}, + {"a recheck ask is held", []BacklogRow{{Language: "go", Kind: "recheck", Held: 1}}, "holds asks"}, {"a stream cannot be read", []BacklogRow{{Language: "go", Kind: "check", Error: "no worker"}}, "cannot be read"}, {"only a delivery ask waits", []BacklogRow{{Language: "go", Kind: "delivery", Waiting: 3}}, ""}} { noAsk.Backlog = func() ([]BacklogRow, error) { return c.rows, nil }