From b6e61fe63da91260b5384023d9f1ede22a03d5d1 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 13 Nov 2026 00:42:25 +0100 Subject: [PATCH 1/2] checker_ask_again ends a lost check that names no ask when no check ask waits in any build seat's stream (issue 1373) Quick fix: a running verdict whose asks state names no ask is asked again when mesh-delivery-view's build_usage backlog shows every check stream with 0 waiting and 0 held and no unreadable row, and no step of the candidate runs; otherwise it refuses, naming the stream that holds asks. Adds the invoke mesh-delivery-view.build_usage to the checker's manifest. Thorough fix (not built): a group check asked while a lone member's own ask ran was swallowed by the checker's claim of one check per candidate; noted on issue 1373. Claude-Session: https://claude.ai/code/session_01NcbgfGKaxw984XyAPfLm4h --- .../cmd/mesh-delivery-checker-tools/main.go | 26 ++++++++++++ .../internal/check/askagain.go | 40 +++++++++++++++++++ .../internal/check/lostask_test.go | 26 ++++++++++++ .../internal/check/read.go | 4 +- .../mesh-delivery-checker/manifest_test.go | 5 +++ modules/mesh-delivery-checker/module.json | 1 + 6 files changed, 101 insertions(+), 1 deletion(-) 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 28504499..4ef85053 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 @@ -471,6 +471,7 @@ func main() { return err }} trying := Trying{Tries: store, Ender: store, Trunks: trunks} + r.Backlog = backlogOf(stdio.Ask) // the build seats' streams, for checker_ask_again (issue 1373) verdicts := stdio.State("verdicts") written := stateStore{entry: func(key string) (json.RawMessage, uint64, error) { reads.Add(1) @@ -496,3 +497,28 @@ func main() { os.Exit(1) } } + +// buildUsageTool is the delivery view's tool of the build nodes' use, whose backlog is the build seats' streams (issue 1373). +const buildUsageTool = "mesh-delivery-view.build_usage" + +// backlogOf reads the build seats' backlog through the view's tool, asked through ask (stdio.Ask, or a test's). +func backlogOf(ask func(string, any) (json.RawMessage, error)) func() ([]check.BacklogRow, error) { + return func() ([]check.BacklogRow, error) { + reads.Add(1) + raw, err := ask(buildUsageTool, map[string]any{}) + if err != nil { + return nil, fmt.Errorf("%s does not answer: %w", buildUsageTool, err) + } + answer, said, ok := buscalls.Unwrap(raw) + if !ok { + return nil, fmt.Errorf("%s answered: %s", buildUsageTool, said) + } + var a struct { + Backlog []check.BacklogRow `json:"backlog"` + } + if err := json.Unmarshal(answer, &a); err != nil { + return nil, fmt.Errorf("%s answered what does not decode: %w", buildUsageTool, err) + } + return a.Backlog, nil + } +} diff --git a/modules/mesh-delivery-checker/internal/check/askagain.go b/modules/mesh-delivery-checker/internal/check/askagain.go index 4a9ec8c5..3a3b0697 100644 --- a/modules/mesh-delivery-checker/internal/check/askagain.go +++ b/modules/mesh-delivery-checker/internal/check/askagain.go @@ -151,6 +151,14 @@ func (r Reader) AskAgain(w VerdictEntry, group, why string) (AskedAgain, error) if err == nil && len(kept) > 0 { err = json.Unmarshal(kept, &put) } + if err == nil && len(put.Asks) == 0 && r.Backlog != nil { + // Issue 1373: the checker names no ask, so the build seats' check streams are read: with no check ask waiting or held + // on any of them (and no step of the candidate running, above) none of it can run, and it is asked anew. + if err := r.noCheckAskWaits(); err != nil { + return AskedAgain{}, err + } + return r.askedAgain(w, key, rev, v, why) + } if err != nil || len(put.Asks) == 0 { return AskedAgain{}, fmt.Errorf("the checker names no ask of %s (%v), so whether one waits in a build seat's stream cannot be told; asking again could run it twice", key, err) } @@ -236,3 +244,35 @@ func (ch *Checker) VerdictWritten(ctx context.Context, key string, value []byte) ch.logf("%s: %s; asked again", key, failedOf(v).Words) ch.consider(ctx, group, cand) } + +// BacklogRow is a row of the build seats' backlog as mesh-delivery-view's build_usage answers it (its backlog): the asks +// waiting and held on one language's stream of one kind of ask, or why it could not be read. +type BacklogRow struct { + Language string `json:"language"` + Kind string `json:"kind"` + Waiting uint64 `json:"waiting"` + Held uint64 `json:"held"` + 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 +// 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() + if err != nil { + 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" { + continue + } + if row.Error != "" { + return fmt.Errorf("the %s build seat's check stream cannot be read (%s), so whether an ask of the check waits in it cannot be told", row.Language, row.Error) + } + if row.Waiting > 0 || row.Held > 0 { + return fmt.Errorf("the %s build seat's check stream holds asks (%d waiting, %d held), one of them may be the check's: asking again could run it twice; "+ + "the seats' backlog is in mesh-delivery-view.build_usage", row.Language, row.Waiting, row.Held) + } + } + return nil +} diff --git a/modules/mesh-delivery-checker/internal/check/lostask_test.go b/modules/mesh-delivery-checker/internal/check/lostask_test.go index 57f3b802..5cc7dbc5 100644 --- a/modules/mesh-delivery-checker/internal/check/lostask_test.go +++ b/modules/mesh-delivery-checker/internal/check/lostask_test.go @@ -327,6 +327,32 @@ func TestACheckWhoseAskJudgedNothingIsAskedAgainOrRead(t *testing.T) { if st, _ := rd.Status(); !strings.Contains(st.Lost["feat/x"], AskAgainTool) { t.Errorf("status lost = %v, want feat/x named with %s", st.Lost, AskAgainTool) } + // Issue 1373: a check that names no ask is asked again only when no check ask waits or is held in any build seat's stream. + { + raw, _, _, _ := r.verdicts.Get(ctx, key) + vcopy := newMemKV() + _, _ = vcopy.Create(ctx, key, raw) + noAsk := rd + noAsk.AskState = kvRead(newMemKV()) + for _, c := range []struct { + name string + rows []BacklogRow + 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 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 } + w := VerdictEntry(asVerdicts{r.verdicts}) + if c.bad == "" { + w = asVerdicts{vcopy} + } + got, err := noAsk.AskAgain(w, "feat/x", "no ask") + if (c.bad == "") != (err == nil) || (c.bad != "" && !strings.Contains(err.Error(), c.bad)) || (c.bad == "" && !strings.HasPrefix(got.Said, AskedAgainWords)) { + t.Errorf("a check naming no ask, %s: %+v, %v", c.name, got, err) + } + } + } got, err := rd.AskAgain(asVerdicts{r.verdicts}, "feat/x", "its watch went deaf") if err != nil || got.Key != key || !strings.HasPrefix(got.Said, AskedAgainWords) { t.Fatalf("asked again = %+v, %v", got, err) diff --git a/modules/mesh-delivery-checker/internal/check/read.go b/modules/mesh-delivery-checker/internal/check/read.go index 54ee1798..2eca0f50 100644 --- a/modules/mesh-delivery-checker/internal/check/read.go +++ b/modules/mesh-delivery-checker/internal/check/read.go @@ -63,7 +63,9 @@ type Reader struct { BuildState StateRead // AskState is the checker's state asks: the asks each running check put (AsksState), which checker_ask_again reads. AskState StateRead - Now func() time.Time + // Backlog is the build seats' backlog (mesh-delivery-view.build_usage); checker_ask_again reads it for a check that names no ask. + Backlog func() ([]BacklogRow, error) + Now func() time.Time } // VerdictRead is a verdict as stored with every failure it names, one line each, and the full report of the status of each diff --git a/modules/mesh-delivery-checker/manifest_test.go b/modules/mesh-delivery-checker/manifest_test.go index 796113f8..d9b4073a 100644 --- a/modules/mesh-delivery-checker/manifest_test.go +++ b/modules/mesh-delivery-checker/manifest_test.go @@ -105,6 +105,11 @@ func sorted(s []string) []string { func TestManifestDeclarations(t *testing.T) { // 1678/AC1 m, top := load(t) + // 1373: checker_ask_again asks the delivery view's build_usage for the build seats' backlog; the list below is the rest. + if !slices.Contains(m.Invokes, "mesh-delivery-view.build_usage") { + t.Error("invokes lack mesh-delivery-view.build_usage, which checker_ask_again asks for the build seats' streams") + } + m.Invokes = slices.DeleteFunc(slices.Clone(m.Invokes), func(s string) bool { return s == "mesh-delivery-view.build_usage" }) if m.Module != "mesh-delivery-checker" { t.Errorf("module = %q", m.Module) } diff --git a/modules/mesh-delivery-checker/module.json b/modules/mesh-delivery-checker/module.json index 96a896b3..8e8ad85e 100644 --- a/modules/mesh-delivery-checker/module.json +++ b/modules/mesh-delivery-checker/module.json @@ -93,6 +93,7 @@ "mesh-delivery-indexer.indexer_read_seat_definitions", "seat:mesh-catalog.seat-definitions", "seat:mesh-config.renames", + "mesh-delivery-view.build_usage", "seat:mesh-hand-acts.hand-act", "seat:node-build-go.drop", "seat:node-build-node.drop", From 2ab74991b856e5a02f3db101eaf94dd94d2656c1 Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 13 Nov 2026 00:48:36 +0100 Subject: [PATCH 2/2] 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 }