From 453cdfa6070cb54977bccff4567a5c91a52597a1 Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 3 Nov 2026 17:51:09 +0100 Subject: [PATCH 1/3] =?UTF-8?q?The=20check=20waits=20on=20the=20builder's?= =?UTF-8?q?=20builds=20of=20its=20candidate=20before=20it=20composes,=20so?= =?UTF-8?q?=20it=20composes=20with=20them=20(ADR=200603=20=C2=A710)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The builder already builds every candidate fingerprint into its own key. The check now watches exactly those keys on the bus and composes once each is done or failed, within the new setting build-wait-bound (1800 s). A failed build fails the check, naming it; a build not ended within the bound is named and the check composes without it. A module built by a build module the change alters (mesh-build-go or mesh-build-node), one standing on a base the change alters, and one the trunk holds no record of are not waited on, and the composition's warnings name each with why. Claude-Session: https://claude.ai/code/session_01JG73KG7AfSdh6FP8VrBqCn --- .../mesh-delivery-checker/integration_test.go | 47 ++++++- .../cmd/mesh-delivery-checker/judge.go | 61 +++++++++- .../cmd/mesh-delivery-checker/main.go | 10 +- .../cmd/mesh-delivery-checker/settings.go | 49 +++++--- .../mesh-delivery-checker/settings_test.go | 27 ++-- .../mesh-delivery-checker/files/settings.json | 3 +- .../internal/check/build_change.go | 91 ++++++++++---- .../internal/check/builds.go | 69 +++++++++++ .../internal/check/composition_test.go | 115 +++++++++++++++--- .../internal/check/judge.go | 8 +- .../mesh-delivery-checker/manifest_test.go | 11 +- modules/mesh-delivery-checker/module.json | 11 +- 12 files changed, 421 insertions(+), 81 deletions(-) diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go index 4a03d309..d2dc3250 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go @@ -398,7 +398,7 @@ func startBed(t *testing.T) *bed { delivery.CheckerOwedBucket: delivery.CheckerOwedHistory, delivery.PreviewsBucket: delivery.PreviewsHistory, delivery.CandidatesBucket: delivery.CandidatesHistory, delivery.ModulesBucket: delivery.ModulesHistory, delivery.ManifestsBucket: delivery.ManifestsHistory, delivery.SourceBucket: delivery.SourceHistory, delivery.PresenceBucket: delivery.PresenceHistory, delivery.HaveBucket: delivery.HaveHistory, check.TriesBucket: 1, - delivery.TurnsBucket: delivery.TurnsHistory} { + delivery.TurnsBucket: delivery.TurnsHistory, delivery.BuilderBuildsBucket: 1} { kv, err := js.CreateKeyValue(ctx, jetstream.KeyValueConfig{Bucket: name, History: uint8(history)}) if err != nil { t.Fatal(err) @@ -578,12 +578,50 @@ func (b *bed) owed(t *testing.T, number int, head, state string) delivery.Owed { // TestCheckerJudgesACandidateAndOwesItsStatus: a candidate the indexer wrote, one pull request of mesh-delivery, is fetched // merged onto the trunk the gate names, digested and tested at both levels on the node, kept, previewed; the verdict passes // with the counts of both levels and the composition not run, and the status mesh/check is owed on the head as success. -func TestCheckerJudgesACandidateAndOwesItsStatus(t *testing.T) { // N6/AC5, 1300/AC5, 1445/AC1, 1929/AC1 +func TestCheckerJudgesACandidateAndOwesItsStatus(t *testing.T) { // N6/AC5, 1300/AC5, 1445/AC1, 1929/AC1, ADR 0603 §10 b := startBed(t) - key := b.candidate(t, "feat/x", pull(repo, 7, head1)) + // ADR 0603 §10: the candidate changes the module m, whose manifest the trunk holds; the builder has not built m's + // candidate fingerprint yet, so the check waits on the builder's key before it composes. + manifest := `{"module":"m"}` + digest := delivery.ManifestDigest([]byte(manifest)) + stored, err := delivery.EncodeManifest(delivery.Manifest{Module: "m", JSON: manifest}) + if err != nil { + t.Fatal(err) + } + mkey, _ := delivery.ManifestsKey(digest) + if _, err := b.buckets[delivery.ManifestsBucket].Put(b.ctx, mkey, stored); err != nil { + t.Fatal(err) + } + known, err := delivery.EncodeModule(delivery.Module{Repository: repo, Commit: trunk, Path: "modules/m", Manifest: digest, None2: "the fingerprint is computed with its chain"}) + if err != nil { + t.Fatal(err) + } + if _, err := b.buckets[delivery.ModulesBucket].Put(b.ctx, "m", known); err != nil { + t.Fatal(err) + } + fp := "src2:" + strings.Repeat("f", 64) + p := pull(repo, 7, head1) + p.Modules, p.Fps = []string{"m"}, map[string]delivery.CandidateModule{"m": {FP2: fp, Manifest: digest}} + key := b.candidate(t, "feat/x", p) // ADR 0593 §7, §9 (D12): n1 and n2 are placed; n1's engine answers compose-candidate on the bus, n2's never does. asked := b.engine(t, "n1", "n2") b.start(t, nil) + b.said(t, "waits on the builder's builds of m before it composes") + if got := asked(); len(got) != 0 { + t.Fatalf("the engines were asked %+v before m's build ended", got) + } + bkey, _ := delivery.BuilderBuildKey("m", fp) + building, _ := json.Marshal(delivery.BuilderBuild{V: 1, State: delivery.BuildBuilding}) + if _, err := b.buckets[delivery.BuilderBuildsBucket].Put(b.ctx, bkey, building); err != nil { + t.Fatal(err) + } + if v := b.verdict(t, key, func(delivery.Verdict) bool { return true }); v.State != delivery.VerdictRunning || len(asked()) != 0 { + t.Fatalf("verdict %s and %d asks while m builds: the check composes only once its build ended", v.State, len(asked())) + } + built, _ := json.Marshal(delivery.BuilderBuild{V: 1, State: delivery.BuildDone, Manifest: digest}) + if _, err := b.buckets[delivery.BuilderBuildsBucket].Put(b.ctx, bkey, built); err != nil { + t.Fatal(err) + } v := b.verdict(t, key, ended) if v.State != delivery.VerdictPass || v.Levels[delivery.LevelUnit].Run != 2 || v.Levels[delivery.LevelIntegration].Run != 2 || v.Trunk[repo] != trunk { t.Fatalf("verdict = %+v", v) @@ -592,6 +630,9 @@ func TestCheckerJudgesACandidateAndOwesItsStatus(t *testing.T) { // N6/AC5, 1300 !strings.Contains(c.Words, "n2: "+delivery.ComposeNoAnswer) || len(v.Machines) != 1 || v.Machines[0] != "n1" { t.Errorf("composition %+v machines %v", v.Composition, v.Machines) } + if v.Composition != nil && strings.Contains(v.Composition.Words, "did not end") { + t.Errorf("the composition says %q: m's build ended within the bound", v.Composition.Words) + } if got := asked(); len(got) == 0 || got[0].Candidate != key { t.Errorf("n1's engine was asked %+v, want the check's candidate %s", got, key) } diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go index 532f5fff..949f4376 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go @@ -8,6 +8,7 @@ import ( "sort" "strings" "sync" + "time" "github.com/nats-io/nats.go/jetstream" @@ -32,6 +33,10 @@ type judgeDeps struct { forge check.Forge announce check.Announcer builds check.BuildStates + // ends waits on the builder's builds of a candidate before the check composes (ADR 0603 §10); nil composes without waiting. + ends check.BuildEnds + // buildWait is the setting build-wait-bound; check.DefaultBuildWait when 0. + buildWait time.Duration // compositions makes a check's composition from the engines' answers (ADR 0593 §7); nil leaves it not run. compositions check.CompositionReader // nodes names every node with a record; nil hands no names. @@ -98,7 +103,7 @@ func newJudge(d judgeDeps) check.Judge { nodes = d.nodes } return &check.Examiner{Nodes: nodes, Compositions: d.compositions, Asks: d.asks, Toolchains: d.tools, - Kept: check.Kept{Store: d.kept}, Previews: d.previews, Modules: d.repos, Packages: d.repos, Published: d.published, Forge: d.forge, Runs: d.repos, Builds: d.builds, Announce: d.announce, Trunks: d.trunks, Manifests: d.repos, Read: d.repos, Predefined: d.predefined, Logf: d.logf} + Kept: check.Kept{Store: d.kept}, Previews: d.previews, Modules: d.repos, Packages: d.repos, Published: d.published, Forge: d.forge, Runs: d.repos, Builds: d.builds, BuildEnds: d.ends, BuildWait: d.buildWait, Announce: d.announce, Trunks: d.trunks, Manifests: d.repos, Read: d.repos, Predefined: d.predefined, Logf: d.logf} } // manifestKV reads a manifest by its digest. @@ -294,6 +299,60 @@ func (r buildReader) Build(ctx context.Context, module, fp2 string) (delivery.Bu return b, err == nil, err } +// buildEnds hears the builder's build keys of a candidate on the bus (mesh-delivery-builder_builds): a watch of exactly +// those keys gives each one's value now and every write the builder makes after, so the check waits on the part that owns +// the fact and polls nothing (novox/hq ADR 0603 §10). +type buildEnds struct{ kv jetstream.KeyValue } + +func (e buildEnds) Ended(ctx context.Context, builds map[string]string) (map[string]delivery.BuilderBuild, error) { + byKey := map[string]string{} + keys := make([]string, 0, len(builds)) + for module, fp2 := range builds { + key, err := delivery.BuilderBuildKey(module, fp2) + if err != nil { + return nil, err + } + byKey[key] = module + keys = append(keys, key) + } + ended := map[string]delivery.BuilderBuild{} + if len(keys) == 0 { + return ended, nil + } + w, err := e.kv.WatchFiltered(ctx, keys) + if err != nil { + return nil, err + } + defer func() { _ = w.Stop() }() + for len(ended) < len(keys) { + select { + case <-ctx.Done(): + return ended, nil // the bound: the builds not ended are left out + case entry, open := <-w.Updates(): + if !open { + return ended, nil + } + if entry == nil || entry.Operation() != jetstream.KeyValuePut { + continue // the end of the values now, or a key deleted: it has not ended + } + module, ours := byKey[entry.Key()] + if !ours { + continue + } + b, err := delivery.DecodeBuilderBuild(entry.Value()) + if err != nil { + continue // not a build the check can read: it has not ended for the check + } + if b.State == delivery.BuildDone || b.State == delivery.BuildFailed { + ended[module] = b + } else { + delete(ended, module) // asked again (builder_build_again): it runs once more + } + } + } + return ended, nil +} + // ManifestDigests is the manifest digest of every module the indexer holds, by module: the catalogue the indexer's judge // judges a change's manifests in (novox/hq ADR 0578 §2). func (r *repoModules) ManifestDigests() map[string]string { diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go index ef544bcb..8ca5d68c 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go @@ -166,14 +166,16 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro // The builder's builds say how the candidates' fingerprints stand (issue 1684); where its bucket is not there yet the line // says nothing of the builds, until the checker starts again. var builds check.BuildStates - if kv := process.OptionalBucket(ctx, js, delivery.BuilderBuildsBucket, "the line says nothing of the builds until the checker starts again", logger.Printf); kv != nil { - builds = buildReader{kvStore{kv}} + var ends check.BuildEnds + if kv := process.OptionalBucket(ctx, js, delivery.BuilderBuildsBucket, "the line says nothing of the builds, and a check composes without waiting on them, until the checker starts again", logger.Printf); kv != nil { + builds, ends = buildReader{kvStore{kv}}, buildEnds{kv} } // The check's composition is the checker's, from what each placed node's engine answers (ADR 0593 §7, §9). - answerBound, err := composeAnswerBound(getenv("MESH_CHECKER_SETTINGS")) + set, err := readSettings(getenv("MESH_CHECKER_SETTINGS")) if err != nil { return err } + answerBound := set.answer holdCallsFor(answerBound) verdicts := &manifestVerdicts{} compositions := check.EngineCompositions{ @@ -212,7 +214,7 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro ch.Judge = newJudge(judgeDeps{ asks: buildrun.Asker{Publisher: asks, Waiter: waiter, Bound: stepBound}, tools: tools, trunks: trunks, kept: kvStore{buckets[delivery.CheckerKeptBucket]}, previews: kvStore{buckets[delivery.PreviewsBucket]}, - repos: repos, published: published, builds: builds, forge: forgeReads{call: caller.Call}, logf: logger.Printf, + repos: repos, published: published, builds: builds, ends: ends, buildWait: set.buildWait, forge: forgeReads{call: caller.Call}, logf: logger.Printf, announce: announce, compositions: compositions, nodes: openNodeNames(ctx, js, logger.Printf), predefined: keptPredefined{process.AwaitBucket(ctx, js, conn, delivery.IndexerModule+"_"+delivery.IndexerPredefinedState, diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings.go index c3c44afd..19d895f3 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings.go @@ -10,38 +10,57 @@ import ( "git.novox.be/novox/mesh-sdk/go/delivery" "git.novox.be/novox/mesh-sdk/go/process" + + "mesh-delivery-checker/internal/check" ) // The checker's own settings (its config, written by the node-engine into ${dir:state}/settings.json, named to the // process by MESH_CHECKER_SETTINGS): compose-answer-bound, the seconds a check waits for a node's engine to answer -// compose-candidate (novox/hq ADR 0593 §7, §9: 60 by default; the checker owns it). +// compose-candidate (novox/hq ADR 0593 §7, §9: 60 by default), and build-wait-bound, the seconds a check waits for the +// builder's builds of its candidate before it composes (ADR 0603 §10: 1800 by default). The checker owns both. -// composeAnswerBound reads compose-answer-bound from the settings file at path: delivery.ComposeAnswerBound where no -// file or no value is there; a value that is no number of seconds from 1 to 600 is refused, naming it. -func composeAnswerBound(path string) (time.Duration, error) { +// settings are the checker's settings as read. +type settings struct { + // answer is compose-answer-bound; buildWait is build-wait-bound. + answer, buildWait time.Duration +} + +// readSettings reads the settings file at path: each value's default where no file or no value is there; a value that is +// no whole number of seconds within its range is refused, naming it. +func readSettings(path string) (settings, error) { + out := settings{answer: delivery.ComposeAnswerBound, buildWait: check.DefaultBuildWait} if path == "" { - return delivery.ComposeAnswerBound, nil + return out, nil } raw, err := os.ReadFile(path) if errors.Is(err, fs.ErrNotExist) { - return delivery.ComposeAnswerBound, nil + return out, nil } if err != nil { - return 0, err + return settings{}, err } var s struct { - Bound *float64 `json:"compose-answer-bound"` + Answer *float64 `json:"compose-answer-bound"` + BuildWait *float64 `json:"build-wait-bound"` } if err := json.Unmarshal(raw, &s); err != nil { - return 0, fmt.Errorf("%s is no settings file: %w", path, err) + return settings{}, fmt.Errorf("%s is no settings file: %w", path, err) } - if s.Bound == nil { - return delivery.ComposeAnswerBound, nil + for _, v := range []struct { + name string + value *float64 + min, max float64 + into *time.Duration + }{{"compose-answer-bound", s.Answer, 1, 600, &out.answer}, {"build-wait-bound", s.BuildWait, 60, 7200, &out.buildWait}} { + if v.value == nil { + continue + } + if *v.value < v.min || *v.value > v.max || *v.value != float64(int(*v.value)) { + return settings{}, fmt.Errorf("%s is %v: a whole number of seconds from %v to %v", v.name, *v.value, v.min, v.max) + } + *v.into = time.Duration(*v.value) * time.Second } - if *s.Bound < 1 || *s.Bound > 600 || *s.Bound != float64(int(*s.Bound)) { - return 0, fmt.Errorf("compose-answer-bound is %v: a whole number of seconds from 1 to 600", *s.Bound) - } - return time.Duration(*s.Bound) * time.Second, nil + return out, nil } // holdCallsFor makes a bus call wait the compose-answer-bound (novox/hq ADR 0593 §7), not the SDK's CallTimeout of 30 s: diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings_test.go index ad9a9863..02e1cc14 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/settings_test.go @@ -8,19 +8,25 @@ import ( "git.novox.be/novox/mesh-sdk/go/delivery" "git.novox.be/novox/mesh-sdk/go/process" + + "mesh-delivery-checker/internal/check" ) // ADR 0593 §7: the answer bound is the checker's setting compose-answer-bound, 60 s when the file or the value is -// absent; a value that is no whole number of seconds from 1 to 600 stops the start, naming it. +// absent; a value that is no whole number of seconds from 1 to 600 stops the start, naming it. ADR 0603 §10: the wait for +// the candidate's builds is build-wait-bound, 30 minutes when absent, from 60 to 7200 seconds. func TestTheAnswerBoundIsTheCheckersSetting(t *testing.T) { dir := t.TempDir() for _, row := range []struct { - body string - want time.Duration - err bool - }{{"", delivery.ComposeAnswerBound, false}, {`{}`, delivery.ComposeAnswerBound, false}, - {`{"compose-answer-bound": 90}`, 90 * time.Second, false}, {`{"compose-answer-bound": 0}`, 0, true}, - {`{"compose-answer-bound": 1.5}`, 0, true}} { + body string + want time.Duration + err bool + buildWait time.Duration + }{{"", delivery.ComposeAnswerBound, false, check.DefaultBuildWait}, {`{}`, delivery.ComposeAnswerBound, false, check.DefaultBuildWait}, + {`{"compose-answer-bound": 90}`, 90 * time.Second, false, check.DefaultBuildWait}, {`{"compose-answer-bound": 0}`, 0, true, 0}, + {`{"compose-answer-bound": 1.5}`, 0, true, 0}, + {`{"compose-answer-bound": 60, "build-wait-bound": 600}`, 60 * time.Second, false, 10 * time.Minute}, + {`{"build-wait-bound": 30}`, 0, true, 0}} { path := filepath.Join(dir, "settings.json") os.Remove(path) if row.body != "" { @@ -28,9 +34,10 @@ func TestTheAnswerBoundIsTheCheckersSetting(t *testing.T) { t.Fatal(err) } } - got, err := composeAnswerBound(path) - if (err != nil) != row.err || got != row.want { - t.Errorf("%q: %s, %v", row.body, got, err) + set, err := readSettings(path) + got := set.answer + if (err != nil) != row.err || got != row.want || set.buildWait != row.buildWait { + t.Errorf("%q: %s, %s, %v", row.body, got, set.buildWait, err) } // Issue 1126: the SDK's call gives up at CallTimeout (30 s), so the checker's calls must wait the bound read // (an answer at 31 s is heard under 60 s), and the bound never shortens the SDK's own. diff --git a/modules/mesh-delivery-checker/files/settings.json b/modules/mesh-delivery-checker/files/settings.json index b9a19671..c6d811e8 100644 --- a/modules/mesh-delivery-checker/files/settings.json +++ b/modules/mesh-delivery-checker/files/settings.json @@ -1,3 +1,4 @@ { - "compose-answer-bound": 60 + "compose-answer-bound": 60, + "build-wait-bound": 1800 } diff --git a/modules/mesh-delivery-checker/internal/check/build_change.go b/modules/mesh-delivery-checker/internal/check/build_change.go index 7065c080..d22339e4 100644 --- a/modules/mesh-delivery-checker/internal/check/build_change.go +++ b/modules/mesh-delivery-checker/internal/check/build_change.go @@ -13,56 +13,99 @@ import ( "git.novox.be/novox/mesh-delivery/sdk/graph" ) -// A change to a build module or a base (novox/hq ADR 0603 §10), as the check can say it. What the check runs is its own -// tests: a candidate Go build module judges its own change (runner workspace, ADR 0535); the check builds no module's -// artifacts, nor a changed base before what stands on it. A base's fingerprint enters its dependents' fingerprints (the -// indexer), so they are other builds once it changes, and the build module's recipe enters the fingerprint of every module -// it builds, so each is built again once the agreed toolchain moves (ADR 0603 §3). The composition composes them from their -// manifests, and the verdict names the changed modules this concerns, as warnings of its composition: words for the -// operator, no part decides on them. +// What the check builds of a candidate before it composes (novox/hq ADR 0603 §10). The builder builds every fingerprint the +// indexer's candidate carries, as a key of its own (builds..), with the build modules that run; the +// check adds no build path of its own and waits on those keys (awaitBuilds), so the node-engines compose the candidate's +// modules with their builds. It does not wait on two kinds of changed module, and names them in its composition's warnings: +// +// - one built by a build module the change alters (the recipe of mesh-build-go or mesh-build-node enters the fingerprint +// of every module it builds, ADR 0530 §1): its candidate build is the running build module's, and the check cannot run +// the candidate's build module to build it (it runs it only for its own digest and tests, ADR 0535 §1), so it is not +// composed with a build of the changed build module; it is built again once the agreed toolchain moves (ADR 0603 §3). +// The build module's own build is waited on, as any other: the running build module builds its new source. +// - one that stands on a module the change alters (build.on): its candidate fingerprint names the base's trunk +// fingerprint, and no build module builds on another module's artifact (ADR 0487 §1), so its build stands on the +// trunk's base; the base's own build is waited on. +// +// A module the trunk holds no record of yet is not built by the builder until it is merged, so it is not waited on either. -// buildChangeWarnings are the composition warnings of a change that alters the Go build module, or a module other changed -// modules are built on: one line each, naming those modules, sorted. None for a change that alters neither, or when the -// manifests cannot be read (nothing is said that was not read). -func (st *state) buildChangeWarnings(ctx context.Context) []string { +// buildsOfTheCheck are the builds the check waits on before it composes (module to fingerprint) and the words for the +// changed modules it composes without waiting on their build, one line each, sorted. A manifest that cannot be read leaves +// every build waited on and nothing said of build modules or bases (nothing is said that was not read). +func (st *state) buildsOfTheCheck(ctx context.Context) (map[string]string, []string) { + wait := map[string]string{} changed := map[string]catalogue.Manifest{} + readable := true for _, p := range st.c.Pulls { for module, cm := range p.Fps { - if cm.FP2 == "" || cm.Manifest == "" || st.j.Read == nil { + if cm.FP2 == "" || cm.Manifest == "" { + continue // the builder builds no fingerprint of it (candidates.go: none of the new form, or no manifest) + } + wait[module] = cm.FP2 + if !readable || st.j.Read == nil { + readable = false continue } m, err := st.j.Read.Manifest(ctx, cm.Manifest) if err != nil { - st.j.logf("%s: the manifest of %s cannot be read, so the modules built with a changed build module or base are not named: %v", st.c.Key, module, err) - return nil + st.j.logf("%s: the manifest of %s cannot be read, so the modules built with a changed build module or base are not named and every build is waited on: %v", st.c.Key, module, err) + readable = false + continue } changed[module] = m } } - var out []string - for _, t := range st.targets { - if t.module != buildrun.GoBuildModule || !changesBuildGo(t.pull.Repository, t.changed) { + var words []string + if st.j.Runs != nil { + var unheld []string + for _, module := range slices.Sorted(maps.Keys(wait)) { + if st.j.Runs.CommitOf(module) == "" { + unheld = append(unheld, module) + delete(wait, module) + } + } + if len(unheld) > 0 { + words = append(words, fmt.Sprintf("%s: the trunk holds no record of it yet, so the builder builds it once it is merged and the check composes it without a build (ADR 0603 §10)", + strings.Join(unheld, ", "))) + } + } + if !readable { + return wait, words + } + for _, bm := range slices.Sorted(maps.Keys(buildrun.LanguageOf)) { + if _, alters := changed[bm]; !alters { continue } var built []string for module, m := range changed { - if m.Build != nil && slices.ContainsFunc(m.Build.Artifacts, func(a catalogue.Artifact) bool { return a.Language == "go" }) { + if _, isBuildModule := buildrun.LanguageOf[module]; !isBuildModule && buildsWith(m, buildrun.LanguageOf[bm]) { built = append(built, module) + delete(wait, module) } } slices.Sort(built) if len(built) > 0 { - out = append(out, fmt.Sprintf("the change alters the build module %s: %s, built with it, are composed from their manifests, not built by the check, and built again once the agreed toolchain moves (ADR 0603 §3, §10)", - buildrun.GoBuildModule, strings.Join(built, ", "))) + words = append(words, fmt.Sprintf("the change alters the build module %s: %s, which it builds, are built by the running %s, not the changed one, which the check cannot run, so the check does not wait on their builds; they are built again once the agreed toolchain moves (ADR 0603 §3, §10)", + bm, strings.Join(built, ", "), bm)) } - break } g := graph.OfManifests(changed) for _, base := range slices.Sorted(maps.Keys(changed)) { if on := g.Dependents(base); len(on) > 0 { - out = append(out, fmt.Sprintf("the change alters %s, a base of %s: their fingerprints name its fingerprint, so they are composed from their manifests, not built by the check (ADR 0603 §10)", - base, strings.Join(on, ", "))) + for _, module := range on { + delete(wait, module) + } + words = append(words, fmt.Sprintf("the change alters %s, a base of %s: their builds stand on the trunk's %s, since no build module builds on a candidate's base, so the check waits on the build of %s alone (ADR 0603 §10)", + base, strings.Join(on, ", "), base, base)) } } - return out + return wait, words +} + +// buildsWith says whether a manifest has a bundle in the language: the artifacts whose fingerprint names that language's +// toolchain and its build module's recipe (an archive is packed with no toolchain, ADR 0478 clause 1). +func buildsWith(m catalogue.Manifest, language string) bool { + return m.Build != nil && slices.ContainsFunc(m.Build.Artifacts, func(a catalogue.Artifact) bool { + return a.Kind == catalogue.ArtifactBundle && a.Language == language + }) } diff --git a/modules/mesh-delivery-checker/internal/check/builds.go b/modules/mesh-delivery-checker/internal/check/builds.go index f70115fb..68e51c7c 100644 --- a/modules/mesh-delivery-checker/internal/check/builds.go +++ b/modules/mesh-delivery-checker/internal/check/builds.go @@ -2,7 +2,12 @@ package check import ( "context" + "fmt" + "maps" + "slices" "sort" + "strings" + "time" "git.novox.be/novox/mesh-sdk/go/delivery" ) @@ -64,3 +69,67 @@ func (j *Examiner) buildNotes(ctx context.Context, c *Case) []BuildNote { } return notes } + +// DefaultBuildWait is how long a check waits for the candidate's builds to end before it composes (the setting +// build-wait-bound, novox/hq ADR 0603 §10): past it the check composes without the builds not ended and names them. +const DefaultBuildWait = 30 * time.Minute + +// BuildEnds waits on the builder's build keys (builds..), hearing each key's changes on the bus as the +// builder writes them: the builder owns the fact, and nothing is polled. +type BuildEnds interface { + // Ended waits until the build of every module at its fingerprint (module to fingerprint) is done or failed, or ctx ends, + // and answers the builds that ended, by module. An error is only builds that cannot be heard at all. + Ended(ctx context.Context, builds map[string]string) (map[string]delivery.BuilderBuild, error) +} + +func (j *Examiner) buildWait() time.Duration { + if j.BuildWait > 0 { + return j.BuildWait + } + return DefaultBuildWait +} + +// awaitBuilds waits for the candidate's builds before the check composes (ADR 0603 §10): the check builds what the builder +// builds of the candidate and composes with it. A failed build fails the check, naming the module and the builder's words; +// a build not ended within the bound is named, and the check composes without it. It answers the words of the composition's +// warnings: what the check composes without waiting on its build, and why. +func (st *state) awaitBuilds(ctx context.Context, v *delivery.Verdict) []string { + wait, words := st.buildsOfTheCheck(ctx) + if len(wait) == 0 { + return words + } + modules := slices.Sorted(maps.Keys(wait)) + if st.j.BuildEnds == nil { + return append(words, fmt.Sprintf("the builds of %s cannot be heard, so the check composes without waiting on them (ADR 0603 §10)", strings.Join(modules, ", "))) + } + bound := st.j.buildWait() + waits := fmt.Sprintf("waits on the builder's builds of %s before it composes, at most %s (ADR 0603 §10)", strings.Join(modules, ", "), bound) + st.c.note(waits) + st.j.logf("%s %s", st.c.Key, waits) + at, cancel := context.WithTimeout(ctx, bound) + ended, err := st.j.BuildEnds.Ended(at, wait) + cancel() + if err != nil { + st.j.logf("%s: the builds of %s cannot be heard: %v", st.c.Key, strings.Join(modules, ", "), err) + return append(words, fmt.Sprintf("the builds of %s cannot be heard (%v), so the check composes without waiting on them (ADR 0603 §10)", strings.Join(modules, ", "), err)) + } + var late []string + for _, module := range modules { + b, done := ended[module] + switch { + case !done: + late = append(late, module) + case b.State == delivery.BuildFailed: + f := delivery.FailedFirst{Test: "build: " + module, Words: fmt.Sprintf("the candidate's build at %s failed (builder_build_again builds it again, then mesh-delivery.recheck asks the check again): %s", wait[module], b.Error)} + if v.State != delivery.VerdictFail { + v.State, v.Failed = delivery.VerdictFail, f + continue + } + addFailure(v, f) + } + } + if len(late) > 0 && ctx.Err() == nil { + words = append(words, fmt.Sprintf("the builds of %s did not end within the build-wait-bound of %s, so the check composes without them (ADR 0603 §10)", strings.Join(late, ", "), bound)) + } + return words +} diff --git a/modules/mesh-delivery-checker/internal/check/composition_test.go b/modules/mesh-delivery-checker/internal/check/composition_test.go index c8dc90c3..53f42d07 100644 --- a/modules/mesh-delivery-checker/internal/check/composition_test.go +++ b/modules/mesh-delivery-checker/internal/check/composition_test.go @@ -104,41 +104,122 @@ type fixedTrunks struct{ heads map[string]string } func (f fixedTrunks) Heads() map[string]string { return f.heads } -// ADR 0603 §10: a change that alters the Go build module, or a module others stand on, names in the composition's warnings -// (and so the check's line) the changed modules built with it, from their manifests; a change that alters neither says nothing. +// fakeEnds answers the builder's builds of a candidate as fakeBuilds holds them, recording the builds waited on; the +// composition must not have been asked yet when it is (ADR 0603 §10: the check composes with the builds). +type fakeEnds struct { + builds fakeBuilds + asked map[string]string + composed *fakeCompositions + early bool +} + +func (f *fakeEnds) Ended(_ context.Context, builds map[string]string) (map[string]delivery.BuilderBuild, error) { + f.asked = builds + f.early = f.composed != nil && len(f.composed.asked) > 0 + out := map[string]delivery.BuilderBuild{} + for module, fp := range builds { + if b, ok := f.builds[module+" "+fp]; ok && (b.State == delivery.BuildDone || b.State == delivery.BuildFailed) { + out[module] = b + } + } + return out, nil +} + +// knownRuns are the modules the trunk holds a record of (the indexer's modules): any commit. +type knownRuns []string + +func (k knownRuns) CommitOf(module string) string { + if slices.Contains(k, module) { + return trunkA + } + return "" +} + +// ADR 0603 §10: before it composes, the check waits on the builder's builds of the candidate's changed modules and composes +// with them; a failed build fails the check naming it, and one not ended within the bound is named. It does not wait on a +// module built by a build module the change alters, nor on one standing on a base the change alters, nor on one the trunk +// holds no record of, and names each in the composition's warnings (and so the check's line); a change that alters none of +// these says nothing. func aChangeToABuildModuleOrBaseNamesTheModulesBuiltOnIt(t *testing.T) { dig := func(c string) string { return strings.Repeat(c, 64) } - goBuild := &catalogue.Build{Artifacts: []catalogue.Artifact{{Name: "bin", Language: "go"}}} + fp := "src2:" + dig("f") + goBuild := &catalogue.Build{Artifacts: []catalogue.Artifact{{Name: "bin", Kind: catalogue.ArtifactBundle, Language: "go"}}} + tsBuild := &catalogue.Build{Artifacts: []catalogue.Artifact{{Name: "app", Kind: catalogue.ArtifactBundle, Language: "typescript"}}} + done := delivery.BuilderBuild{State: delivery.BuildDone} for _, row := range []struct { name string - changed []string manifest map[string]catalogue.Manifest + builds map[string]delivery.BuilderBuild // by module, at fp; nil leaves the check without the builds' ends + runs knownRuns // nil: the trunk holds every module want []string + waited string // the modules waited on, sorted + failed string // the failure's test, when the check fails }{ - {name: "the build module changes", changed: []string{buildGoFile}, - manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}, "mail": {Module: "mail", Build: goBuild}, "docs": {Module: "docs"}}, - want: []string{"mesh-build-go: mail, web, built with it"}}, - {name: "a base changes", changed: []string{"README.md"}, + {name: "the Go build module changes", + manifest: map[string]catalogue.Manifest{"mesh-build-go": {Module: "mesh-build-go", Build: goBuild}, "web": {Module: "web", Build: goBuild}, + "mail": {Module: "mail", Build: goBuild}, "docs": {Module: "docs"}}, + builds: map[string]delivery.BuilderBuild{"mesh-build-go": done, "docs": done}, + want: []string{"the build module mesh-build-go: mail, web, which it builds"}, waited: "docs, mesh-build-go"}, + {name: "the Node build module changes", + manifest: map[string]catalogue.Manifest{"mesh-build-node": {Module: "mesh-build-node", Build: goBuild}, "panel": {Module: "panel", Build: tsBuild}, + "web": {Module: "web", Build: goBuild}}, + builds: map[string]delivery.BuilderBuild{"mesh-build-node": done, "web": done}, + want: []string{"the build module mesh-build-node: panel, which it builds"}, waited: "mesh-build-node, web"}, + {name: "a base changes", manifest: map[string]catalogue.Manifest{"image": {Module: "image"}, "web": {Module: "web", Build: &catalogue.Build{On: []catalogue.BuildsOn{{Arg: "BASE", Module: "image", Artifact: "img"}}}}}, - want: []string{"alters image, a base of web"}}, - {name: "neither changes", changed: []string{"README.md"}, - manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}}}, + builds: map[string]delivery.BuilderBuild{"image": done}, + want: []string{"alters image, a base of web", "waits on the build of image alone"}, waited: "image"}, + {name: "neither changes", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}}, + builds: map[string]delivery.BuilderBuild{"web": done}, waited: "web"}, + {name: "a build fails", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}, "mail": {Module: "mail", Build: goBuild}}, + builds: map[string]delivery.BuilderBuild{"web": {State: delivery.BuildFailed, Error: "go build: undefined: x"}, "mail": done}, + waited: "mail, web", failed: "build: web"}, + {name: "a build does not end within the bound", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}}, + builds: map[string]delivery.BuilderBuild{"web": {State: delivery.BuildBuilding}}, + want: []string{"the builds of web did not end within the build-wait-bound"}, waited: "web"}, + {name: "a module the trunk does not hold", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}, "new": {Module: "new", Build: goBuild}}, + builds: map[string]delivery.BuilderBuild{"web": done}, runs: knownRuns{"web"}, + want: []string{"new: the trunk holds no record of it yet"}, waited: "web"}, + {name: "the builds cannot be heard", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}}, + want: []string{"the builds of web cannot be heard"}}, } { b := candidateBench(t, true) - b.fetch.Changed = row.changed - b.ex.Compositions = &fakeCompositions{v: delivery.CompositionVerdict{Group: "feat/x", Heads: "0123456789ab", State: delivery.CompositionPass, SDK: "x", Machines: []string{"g14"}}} + comps := &fakeCompositions{v: delivery.CompositionVerdict{Group: "feat/x", Heads: "0123456789ab", State: delivery.CompositionPass, SDK: "x", Machines: []string{"g14"}}} + b.ex.Compositions = comps + var ends *fakeEnds + if row.builds != nil { + ends = &fakeEnds{builds: fakeBuilds{}, composed: comps} + for module, build := range row.builds { + ends.builds[module+" "+fp] = build + } + b.ex.BuildEnds = ends + } + if row.runs != nil { + b.ex.Runs = row.runs + } read, fps := fixedManifests{}, map[string]delivery.CandidateModule{} for module, m := range row.manifest { d := dig(string(rune('a' + len(read)))) - read[d], fps[module] = m, delivery.CandidateModule{FP2: "src2:" + dig("f"), Manifest: d} + read[d], fps[module] = m, delivery.CandidateModule{FP2: fp, Manifest: d} } b.ex.Read = read p := pullOf(catalogueRepository, 9, head1) p.Fps, p.Modules = fps, slices.Sorted(maps.Keys(fps)) v := b.judge(t, caseOf(t, p)).Verdict - if v.State != delivery.VerdictPass || v.Composition == nil { - t.Fatalf("%s: %+v", row.name, v) + if v.Composition == nil || len(comps.asked) != 1 { + t.Fatalf("%s: composed %d times: %+v", row.name, len(comps.asked), v) + } + if ends != nil { + if got := strings.Join(slices.Sorted(maps.Keys(ends.asked)), ", "); got != row.waited || ends.early { + t.Errorf("%s: waited on %q (after the composition: %v), want %q before it composes", row.name, got, ends.early, row.waited) + } + } + switch { + case row.failed != "" && (v.State != delivery.VerdictFail || v.Failed.Test != row.failed || !strings.Contains(v.Failed.Words, "undefined: x")): + t.Errorf("%s: the verdict is %s failing %+v, want it failed naming %s and the builder's words", row.name, v.State, v.Failed, row.failed) + case row.failed == "" && v.State != delivery.VerdictPass: + t.Errorf("%s: %+v", row.name, v) } words := compositionWords(v.Composition, v.Took.Composition) for _, w := range row.want { @@ -147,7 +228,7 @@ func aChangeToABuildModuleOrBaseNamesTheModulesBuiltOnIt(t *testing.T) { } } if row.want == nil && v.Composition.Words != "" { - t.Errorf("%s: warned %q though neither a build module nor a base changed", row.name, v.Composition.Words) + t.Errorf("%s: warned %q though the check waited on every build of the change", row.name, v.Composition.Words) } } } diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index f15c9429..cb77a6e5 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -102,6 +102,11 @@ type Examiner struct { // Builds says how the build of a fingerprint the candidate carries stands (the builder's builds bucket); nil says nothing // of the builds. Builds BuildStates + // BuildEnds waits on the builder's builds of the candidate before the check composes (ADR 0603 §10); nil composes + // without waiting, said in the composition's warnings. + BuildEnds BuildEnds + // BuildWait is how long the check waits for those builds (the setting build-wait-bound); DefaultBuildWait when 0. + BuildWait time.Duration // Announce says a preview was written under the key (EventPreviewWritten); nil says nothing. Announce Announcer // Packages names, of a repository, the paths (repository-relative) its TypeScript package artifacts are built from, from @@ -280,6 +285,7 @@ func (st *state) composed(ctx context.Context, v *delivery.Verdict) { strings.Join(st.unpreviewed, ", ") + " (its head does not merge or its fetch did not answer), so the change cannot be composed"} return } + words := st.awaitBuilds(ctx, v) // ADR 0603 §10: the check composes with the candidate's builds comp, err := st.j.Compositions.Compose(ctx, st.c) if err != nil { // Said in the verdict, never left as a composition not yet run (review finding 4). @@ -287,7 +293,7 @@ func (st *state) composed(ctx context.Context, v *delivery.Verdict) { v.Composition = &delivery.Composition{State: delivery.CompositionNotRun, Words: "not made: " + err.Error()} return } - comp.Warnings = append(comp.Warnings, st.buildChangeWarnings(ctx)...) // ADR 0603 §10 + comp.Warnings = append(comp.Warnings, words...) // ADR 0603 §10 withComposition(v, comp) if v.Composition != nil && v.Composition.State != delivery.CompositionNotRun { v.Composition.FPSet = st.c.FPSet // a carry keeps it while the fingerprints stay the same (ADR 0548 §5) diff --git a/modules/mesh-delivery-checker/manifest_test.go b/modules/mesh-delivery-checker/manifest_test.go index 7d995a3d..6a379ebe 100644 --- a/modules/mesh-delivery-checker/manifest_test.go +++ b/modules/mesh-delivery-checker/manifest_test.go @@ -174,13 +174,16 @@ func TestManifestProcess(t *testing.T) { // 1678/AC1 if len(env) != 2 || env["MESH_BROKER_FILE"] != "${dir:state}/broker" || env["MESH_CHECKER_SETTINGS"] != "${dir:state}/settings.json" { t.Errorf("env = %v, want the broker file and the settings file", p["env"]) } - // Its settings hold its answer bound alone (ADR 0593 §7: compose-answer-bound, 60 s, the checker's), never a capacity - // of its own (ADR 0525 §4). + // Its settings hold its answer bound (ADR 0593 §7: compose-answer-bound, 60 s, the checker's) and its wait for the + // candidate's builds (ADR 0603 §10: build-wait-bound, 1800 s), never a capacity of its own (ADR 0525 §4). for _, r := range m.Resources { - if r["id"] == "settings" && !reflect.DeepEqual(r["takes-config"], []any{"compose-answer-bound"}) { - t.Errorf("the settings resource %v takes more than compose-answer-bound", r) + if r["id"] == "settings" && !reflect.DeepEqual(r["takes-config"], []any{"compose-answer-bound", "build-wait-bound"}) { + t.Errorf("the settings resource %v takes other than compose-answer-bound and build-wait-bound", r) } } + if c, ok := m.Config["build-wait-bound"]; !ok || c.Default != float64(1800) || c.Trusted { + t.Errorf("build-wait-bound = %+v, want 1800 by default, not trusted", c) + } if c, ok := m.Config["compose-answer-bound"]; !ok || c.Default != float64(60) || c.Trusted { t.Errorf("compose-answer-bound = %+v, want 60 by default, not trusted", c) } diff --git a/modules/mesh-delivery-checker/module.json b/modules/mesh-delivery-checker/module.json index 767884cb..537551f1 100644 --- a/modules/mesh-delivery-checker/module.json +++ b/modules/mesh-delivery-checker/module.json @@ -81,6 +81,14 @@ "maximum": 600, "trusted": false, "default": 60 + }, + "build-wait-bound": { + "type": "number", + "description": "seconds a check waits for the builder's builds of its candidate to end before it composes with them (novox/hq ADR 0603 §10); a build failed fails the check, naming it, and a build not ended by then is named in the verdict and the check composes without it.", + "minimum": 60, + "maximum": 7200, + "trusted": false, + "default": 1800 } }, "resources": [ @@ -106,7 +114,8 @@ "mode": "0600", "owner": "mesh-delivery-checker", "takes-config": [ - "compose-answer-bound" + "compose-answer-bound", + "build-wait-bound" ], "merge": "json", "template": "files/settings.json" From 877b09e92b308d52b1cf6a6d80f8916f14cb1a6d Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 3 Nov 2026 17:51:09 +0100 Subject: [PATCH 2/3] =?UTF-8?q?checker=5Fstatus=20names=20the=20builds=20a?= =?UTF-8?q?=20running=20check=20waits=20on=20(ADR=200603=20=C2=A710)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Worked out from the builder's builds state at each read, stored nowhere: of each running check, the candidate's builds that are neither done nor failed. Claude-Session: https://claude.ai/code/session_01JG73KG7AfSdh6FP8VrBqCn --- .../cmd/mesh-delivery-checker-tools/main.go | 6 ++- .../internal/check/read.go | 49 +++++++++++++++++++ .../internal/check/read_test.go | 21 +++++++- 3 files changed, 72 insertions(+), 4 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 2159d0e5..371003d2 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 @@ -244,7 +244,9 @@ func Tools(r check.Reader) []stdio.Tool { "build seats' streams, whose backlog the delivery view shows (novox/hq ADR 0525). held_by_turn names the candidates whose check an approved " + "merge's turn holds off (novox/hq ADR 0548 §3), with the repository and the merge that waits; the gate's turns tool reads every turn and its " + "hand-now tool steers one. dead_letters says whether the checker hears the asks the build seats give up on (issue 734), and why not: " + - "deaf, a try or check whose ask is given up waits until its bound. Read-only.", + "deaf, a try or check whose ask is given up waits until its bound. builds_not_ended names, of each running check, the builder's builds of " + + "its candidate's changed modules that are neither done nor failed: the check waits on them before it composes, within its build-wait-bound " + + "(novox/hq ADR 0603 §10); a failed one fails the check: the builder's builder_build_again builds it again, then mesh-delivery.recheck asks the check again. Read-only.", Input: map[string]any{"type": "object", "properties": map[string]any{}}, Run: func(map[string]any) (any, error) { return r.Status() }}, } @@ -283,7 +285,7 @@ func main() { StepStates: map[string]check.StateRead{"mesh-build-go": read(stdio.State("mesh-build-go.steps")), "mesh-build-node": read(stdio.State("mesh-build-node.steps"))}, HaveState: read(stdio.State("node-engine.have")), StepBound: stepBound(), TurnState: read(stdio.State("mesh-delivery-gate.turns")), HearingState: read(stdio.State(buildrun.HearingState)), - AskState: read(stdio.State(check.AsksState))} + AskState: read(stdio.State(check.AsksState)), BuildState: read(stdio.State("mesh-delivery-builder.builds"))} r.Runs = runsOf{read(stdio.State("mesh-delivery-indexer.modules"))} // the node-engine at the commit the mesh runs, as the checker reads it tries := stdio.State(check.TriesState) source := stdio.State("mesh-delivery-gate.source") diff --git a/modules/mesh-delivery-checker/internal/check/read.go b/modules/mesh-delivery-checker/internal/check/read.go index 90e57f28..2086eced 100644 --- a/modules/mesh-delivery-checker/internal/check/read.go +++ b/modules/mesh-delivery-checker/internal/check/read.go @@ -3,6 +3,7 @@ package check import ( "encoding/json" "fmt" + "slices" "sort" "strings" "time" @@ -47,6 +48,9 @@ type Reader struct { Runs interface{ CommitOf(module string) string } // HearingState is the checker's state hearing: whether it hears DEAD_LETTERS (issue 734). HearingState StateRead + // BuildState is the builder's builds (mesh-delivery-builder.builds): the builds of a running check's candidate that have + // not ended, which the check waits on before it composes (ADR 0603 §10); without it none is named. + 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 @@ -344,6 +348,10 @@ type Status struct { // Lost are the candidates whose verdict is running while no step of theirs can still run and their newest step ended, by // group, with that step: nothing will judge them (the lost check of 2026-10-28); checker_ask_again asks one again. Lost map[string]string `json:"lost"` + // BuildsNotEnded are, of each candidate whose check runs, the builder's builds of its changed modules that are neither + // done nor failed, by group: ` `, sorted (no key yet is `not begun`). A check waits on them before it + // composes (ADR 0603 §10), but for the modules its line names as composed without them; builder_build_again builds a failed one again. + BuildsNotEnded map[string]string `json:"builds_not_ended,omitempty"` // DeadLetters says whether the checker hears the asks the build seats give up on (issue 734): deaf, a try or check // whose ask is given up waits until its bound. DeadLetters DeadLettersHeard `json:"dead_letters"` @@ -500,6 +508,12 @@ func (r Reader) Status() (Status, error) { if why := read.lostWhy(c); why != "" { st.Lost[c.Group] = why } + if words := r.buildsNotEnded(c); words != "" { + if st.BuildsNotEnded == nil { + st.BuildsNotEnded = map[string]string{} + } + st.BuildsNotEnded[c.Group] = words + } case IsNodesFailure(v): st.Waiting[KindNodes] = append(st.Waiting[KindNodes], c.Group) case IsUntrunked(v): @@ -663,3 +677,38 @@ func (r Reader) leftWhy(module string, step delivery.Step, now time.Time, bound } return "" } + +// buildsNotEnded are the builder's builds of the candidate's changed modules that are neither done nor failed, as +// Status.BuildsNotEnded says them; empty when none, or when the builder's builds are not readable here. +func (r Reader) buildsNotEnded(c delivery.Candidate) string { + if r.BuildState.Get == nil { + return "" + } + var out []string + for _, p := range c.Pulls { + for module, cm := range p.Fps { + if cm.FP2 == "" || cm.Manifest == "" { + continue + } + key, err := delivery.BuilderBuildKey(module, cm.FP2) + if err != nil { + continue + } + raw, err := r.BuildState.Get(key) + if err != nil { + continue + } + state := "not begun" + if len(raw) > 0 { + b, err := delivery.DecodeBuilderBuild(raw) + if err != nil || b.State == delivery.BuildDone || b.State == delivery.BuildFailed { + continue + } + state = b.State + } + out = append(out, module+" "+state) + } + } + sort.Strings(out) + return strings.Join(slices.Compact(out), ", ") +} diff --git a/modules/mesh-delivery-checker/internal/check/read_test.go b/modules/mesh-delivery-checker/internal/check/read_test.go index 9a866222..033ba635 100644 --- a/modules/mesh-delivery-checker/internal/check/read_test.go +++ b/modules/mesh-delivery-checker/internal/check/read_test.go @@ -147,10 +147,24 @@ func TestTheStatusToolReadsRunningSteps(t *testing.T) { passed, _ := delivery.EncodeStep(delivery.Step{ID: "abc2", Tool: "build_go_test", Node: "ace", State: delivery.StepPassed}) cand := candidateOf(t, "feat/x", pullOf(repoA, 7, head1)) ckey, _ := delivery.CandidatesKey("feat/x") + // ADR 0603 §10: feat/y's check runs and waits on the builds of its changed modules: web building, mail not begun; api + // is done and named nowhere. + fp, man := "src2:"+strings.Repeat("f", 64), strings.Repeat("e", 64) + py := pullOf(repoA, 8, head2) + py.Modules, py.Fps = []string{"api", "mail", "web"}, map[string]delivery.CandidateModule{"web": {FP2: fp, Manifest: man}, "mail": {FP2: fp, Manifest: man}, "api": {FP2: fp, Manifest: man}} + candY := candidateOf(t, "feat/y", py) + ykey, _ := delivery.CandidatesKey("feat/y") + runningV, _ := delivery.EncodeVerdict(delivery.Verdict{Group: "feat/y", Heads: "0123456789ab", State: delivery.VerdictRunning}) + builds := fakeState{} + for module, state := range map[string]string{"web": delivery.BuildBuilding, "api": delivery.BuildDone} { + k, _ := delivery.BuilderBuildKey(module, fp) + builds[k], _ = json.Marshal(delivery.BuilderBuild{V: 1, State: state}) + } rd := Reader{ StepStates: map[string]StateRead{"mesh-build-go": fakeState{"shanks.abc1": running, "ace.abc2": passed}.read()}, - CandidateState: fakeState{ckey: cand}.read(), - VerdictState: fakeState{}.read(), + CandidateState: fakeState{ckey: cand, ykey: candY}.read(), + VerdictState: fakeState{keyOf(t, "feat/y", candY): runningV}.read(), + BuildState: builds.read(), HaveState: fakeState{"shanks.mesh-build-go": haveOf(t, delivery.PhaseRunning)}.read(), HearingState: fakeState{"dead-letters": []byte(`{"heard":false,"streams":["SEAT_NODE_BUILD_GO"],` + `"why":"DEAD_LETTERS is not heard, so an ask a build seat gives up on is waited for until its bound: SEAT_NODE_BUILD_GO: permissions violation",` + @@ -167,6 +181,9 @@ func TestTheStatusToolReadsRunningSteps(t *testing.T) { if len(st.Unjudged) != 1 || st.Unjudged[0] != "feat/x" { t.Errorf("unjudged = %v", st.Unjudged) } + if got := st.BuildsNotEnded; len(got) != 1 || got["feat/y"] != "mail not begun, web building" { + t.Errorf("builds not ended = %v, want feat/y's mail not begun and web building", got) + } // Issue 734, review round 1: a checker that does not hear DEAD_LETTERS says so in its status, with why. if st.DeadLetters.Heard || !strings.Contains(st.DeadLetters.Why, "permissions violation") { t.Errorf("dead letters = %+v, want not heard, saying why", st.DeadLetters) From 3842a464ed9b577c3e352ded6d06b45d85f817f6 Mon Sep 17 00:00:00 2001 From: jochen Date: Tue, 3 Nov 2026 17:55:41 +0100 Subject: [PATCH 3/3] =?UTF-8?q?A=20check=20failed=20on=20a=20build=20asks?= =?UTF-8?q?=20itself=20again=20once=20the=20build=20is=20done=20again,=20a?= =?UTF-8?q?nd=20a=20failed=20check=20never=20waits=20on=20builds=20(ADR=20?= =?UTF-8?q?0603=20=C2=A710)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A failed candidate build no longer needs mesh-delivery.recheck, an exception verb (ADR 0421 §12): the checker hears the builder's keys the failed verdict names and, once each is written done (builder_build_again), retakes the check of the same heads as it does after a failure of the nodes, at its start too. When the tests have already failed the check composes at once and names the builds it did not wait on, so its red status is never held back by a build. Claude-Session: https://claude.ai/code/session_01JG73KG7AfSdh6FP8VrBqCn --- .../cmd/mesh-delivery-checker-tools/main.go | 2 +- .../mesh-delivery-checker/integration_test.go | 20 ++++- .../cmd/mesh-delivery-checker/judge.go | 33 +++++--- .../cmd/mesh-delivery-checker/main.go | 3 + .../internal/check/builds.go | 37 ++++++++- .../internal/check/checker.go | 77 ++++++++++++++++++- .../internal/check/composition_test.go | 24 +++++- .../internal/check/read.go | 2 +- 8 files changed, 173 insertions(+), 25 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 371003d2..fd39fe86 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 @@ -246,7 +246,7 @@ func Tools(r check.Reader) []stdio.Tool { "hand-now tool steers one. dead_letters says whether the checker hears the asks the build seats give up on (issue 734), and why not: " + "deaf, a try or check whose ask is given up waits until its bound. builds_not_ended names, of each running check, the builder's builds of " + "its candidate's changed modules that are neither done nor failed: the check waits on them before it composes, within its build-wait-bound " + - "(novox/hq ADR 0603 §10); a failed one fails the check: the builder's builder_build_again builds it again, then mesh-delivery.recheck asks the check again. Read-only.", + "(novox/hq ADR 0603 §10); a failed one fails the check: rebuild it with the builder's builder_build_again, and the check asks itself again when the build is done. Read-only.", Input: map[string]any{"type": "object", "properties": map[string]any{}}, Run: func(map[string]any) (any, error) { return r.Status() }}, } diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go index d2dc3250..823deb6c 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/integration_test.go @@ -618,12 +618,23 @@ func TestCheckerJudgesACandidateAndOwesItsStatus(t *testing.T) { // N6/AC5, 1300 if v := b.verdict(t, key, func(delivery.Verdict) bool { return true }); v.State != delivery.VerdictRunning || len(asked()) != 0 { t.Fatalf("verdict %s and %d asks while m builds: the check composes only once its build ended", v.State, len(asked())) } + // m's build fails: the check fails naming it. Built again (builder_build_again writes the key done), the check asks + // itself again for the same heads, with no recheck, and passes. + failedBuild, _ := json.Marshal(delivery.BuilderBuild{V: 1, State: delivery.BuildFailed, Error: "go build: undefined: x"}) + if _, err := b.buckets[delivery.BuilderBuildsBucket].Put(b.ctx, bkey, failedBuild); err != nil { + t.Fatal(err) + } + if v := b.verdict(t, key, ended); v.State != delivery.VerdictFail || v.Failed.Test != check.BuildFailedTest+"m" { + t.Fatalf("verdict %+v, want it failed on m's build", v) + } built, _ := json.Marshal(delivery.BuilderBuild{V: 1, State: delivery.BuildDone, Manifest: digest}) if _, err := b.buckets[delivery.BuilderBuildsBucket].Put(b.ctx, bkey, built); err != nil { t.Fatal(err) } - v := b.verdict(t, key, ended) - if v.State != delivery.VerdictPass || v.Levels[delivery.LevelUnit].Run != 2 || v.Levels[delivery.LevelIntegration].Run != 2 || v.Trunk[repo] != trunk { + b.said(t, "the builds of m it failed on are done again, so it is checked again") + v := b.verdict(t, key, func(v delivery.Verdict) bool { return v.State == delivery.VerdictPass }) + // The check asked again takes the packages the failed check ran from their kept verdicts. + if v.State != delivery.VerdictPass || v.Levels[delivery.LevelUnit].Kept != 2 || v.Levels[delivery.LevelIntegration].Kept != 2 || v.Trunk[repo] != trunk { t.Fatalf("verdict = %+v", v) } if c := v.Composition; c == nil || c.State != delivery.CompositionPass || !slices.Equal(c.Machines, []string{"n1"}) || @@ -643,10 +654,11 @@ func TestCheckerJudgesACandidateAndOwesItsStatus(t *testing.T) { // N6/AC5, 1300 t.Errorf("the check ask named the nodes %v, want [n1] from the have's node record", names) } o := b.owed(t, 7, head1, "success") - if o.Context != delivery.StatusContextCheck || !strings.HasPrefix(o.Text, "pass 111111111111 on aaaaaaaaaaaa") || !strings.Contains(o.Text, "unit 2 run") { + if o.Context != delivery.StatusContextCheck || !strings.HasPrefix(o.Text, "pass 111111111111 on aaaaaaaaaaaa") || !strings.Contains(o.Text, "unit 0 run, 2 kept") { t.Errorf("owed = %+v", o) } - for tool, want := range map[string]int{"mesh-build-go.build_go_fetch": 1, "mesh-build-go.build_go_digest": 2, "mesh-build-go.build_go_test": 2} { + // Two checks of the same heads: the one failed on m's build and the one it asked itself again; the second runs no test. + for tool, want := range map[string]int{"mesh-build-go.build_go_fetch": 2, "mesh-build-go.build_go_digest": 4, "mesh-build-go.build_go_test": 2} { if got := b.fake.count(tool); got != want { t.Errorf("%s called %d times, want %d", tool, got, want) } diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go index 949f4376..047e9ab0 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/judge.go @@ -305,6 +305,18 @@ func (r buildReader) Build(ctx context.Context, module, fp2 string) (delivery.Bu type buildEnds struct{ kv jetstream.KeyValue } func (e buildEnds) Ended(ctx context.Context, builds map[string]string) (map[string]delivery.BuilderBuild, error) { + return e.until(ctx, builds, func(b delivery.BuilderBuild) bool { + return b.State == delivery.BuildDone || b.State == delivery.BuildFailed + }) +} + +func (e buildEnds) Done(ctx context.Context, builds map[string]string) bool { + done, err := e.until(ctx, builds, func(b delivery.BuilderBuild) bool { return b.State == delivery.BuildDone }) + return err == nil && len(done) == len(builds) +} + +// until hears the builds' keys until each holds a build that ended holds, or ctx ends, and answers those that do, by module. +func (e buildEnds) until(ctx context.Context, builds map[string]string, ended func(delivery.BuilderBuild) bool) (map[string]delivery.BuilderBuild, error) { byKey := map[string]string{} keys := make([]string, 0, len(builds)) for module, fp2 := range builds { @@ -315,22 +327,22 @@ func (e buildEnds) Ended(ctx context.Context, builds map[string]string) (map[str byKey[key] = module keys = append(keys, key) } - ended := map[string]delivery.BuilderBuild{} + out := map[string]delivery.BuilderBuild{} if len(keys) == 0 { - return ended, nil + return out, nil } w, err := e.kv.WatchFiltered(ctx, keys) if err != nil { return nil, err } defer func() { _ = w.Stop() }() - for len(ended) < len(keys) { + for len(out) < len(keys) { select { case <-ctx.Done(): - return ended, nil // the bound: the builds not ended are left out + return out, nil // the bound: the builds not ended are left out case entry, open := <-w.Updates(): if !open { - return ended, nil + return out, nil } if entry == nil || entry.Operation() != jetstream.KeyValuePut { continue // the end of the values now, or a key deleted: it has not ended @@ -340,17 +352,14 @@ func (e buildEnds) Ended(ctx context.Context, builds map[string]string) (map[str continue } b, err := delivery.DecodeBuilderBuild(entry.Value()) - if err != nil { - continue // not a build the check can read: it has not ended for the check - } - if b.State == delivery.BuildDone || b.State == delivery.BuildFailed { - ended[module] = b + if err == nil && ended(b) { + out[module] = b } else { - delete(ended, module) // asked again (builder_build_again): it runs once more + delete(out, module) // asked again (builder_build_again), or not a build the check can read: not ended } } } - return ended, nil + return out, nil } // ManifestDigests is the manifest digest of every module the indexer holds, by module: the catalogue the indexer's judge diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go index 8ca5d68c..1cc2b977 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go @@ -170,6 +170,9 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro if kv := process.OptionalBucket(ctx, js, delivery.BuilderBuildsBucket, "the line says nothing of the builds, and a check composes without waiting on them, until the checker starts again", logger.Printf); kv != nil { builds, ends = buildReader{kvStore{kv}}, buildEnds{kv} } + if ends != nil { + ch.Rebuilds = ends // a check failed on a build is asked again once that build is done again (ADR 0603 §10) + } // The check's composition is the checker's, from what each placed node's engine answers (ADR 0593 §7, §9). set, err := readSettings(getenv("MESH_CHECKER_SETTINGS")) if err != nil { diff --git a/modules/mesh-delivery-checker/internal/check/builds.go b/modules/mesh-delivery-checker/internal/check/builds.go index 68e51c7c..2afa2d42 100644 --- a/modules/mesh-delivery-checker/internal/check/builds.go +++ b/modules/mesh-delivery-checker/internal/check/builds.go @@ -80,6 +80,37 @@ type BuildEnds interface { // Ended waits until the build of every module at its fingerprint (module to fingerprint) is done or failed, or ctx ends, // and answers the builds that ended, by module. An error is only builds that cannot be heard at all. Ended(ctx context.Context, builds map[string]string) (map[string]delivery.BuilderBuild, error) + // Done waits until the build of every module at its fingerprint is done (a failed one built again, builder_build_again), + // and says whether they all are; false when ctx ends first or they cannot be heard. + Done(ctx context.Context, builds map[string]string) bool +} + +// BuildFailedTest is what a failure of the check names a failed build of the candidate by: the test `build: `. +// A verdict failed on builds is asked again by the checker itself once each is built again (Checker.Rebuilt). +const BuildFailedTest = "build: " + +// failedBuilds are the candidate's builds a failed verdict names as failed (BuildFailedTest), by module, at the fingerprint +// the case carries for it; none for a verdict that names no failed build. +func failedBuilds(v delivery.Verdict, cs *Case) map[string]string { + if v.State != delivery.VerdictFail { + return nil + } + out := map[string]string{} + for _, f := range append([]delivery.FailedFirst{v.Failed}, v.Failures...) { + module, named := strings.CutPrefix(f.Test, BuildFailedTest) + if !named { + continue + } + for _, p := range cs.Pulls { + if cm, ok := p.Fps[module]; ok && cm.FP2 != "" { + out[module] = cm.FP2 + } + } + } + if len(out) == 0 { + return nil + } + return out } func (j *Examiner) buildWait() time.Duration { @@ -99,6 +130,10 @@ func (st *state) awaitBuilds(ctx context.Context, v *delivery.Verdict) []string return words } modules := slices.Sorted(maps.Keys(wait)) + if v.State == delivery.VerdictFail { + // The check has failed already: its red status is never held back by a build (the lead, 2026-11-03). + return append(words, fmt.Sprintf("the check failed before its builds were waited on, so it composes at once, without waiting on the builds of %s (ADR 0603 §10)", strings.Join(modules, ", "))) + } if st.j.BuildEnds == nil { return append(words, fmt.Sprintf("the builds of %s cannot be heard, so the check composes without waiting on them (ADR 0603 §10)", strings.Join(modules, ", "))) } @@ -120,7 +155,7 @@ func (st *state) awaitBuilds(ctx context.Context, v *delivery.Verdict) []string case !done: late = append(late, module) case b.State == delivery.BuildFailed: - f := delivery.FailedFirst{Test: "build: " + module, Words: fmt.Sprintf("the candidate's build at %s failed (builder_build_again builds it again, then mesh-delivery.recheck asks the check again): %s", wait[module], b.Error)} + f := delivery.FailedFirst{Test: BuildFailedTest + module, Words: fmt.Sprintf("the candidate's build at %s failed (rebuild with builder_build_again; the check asks itself again when the build is done): %s", wait[module], b.Error)} if v.State != delivery.VerdictFail { v.State, v.Failed = delivery.VerdictFail, f continue diff --git a/modules/mesh-delivery-checker/internal/check/checker.go b/modules/mesh-delivery-checker/internal/check/checker.go index 3fbffddb..4e8e91bc 100644 --- a/modules/mesh-delivery-checker/internal/check/checker.go +++ b/modules/mesh-delivery-checker/internal/check/checker.go @@ -4,6 +4,8 @@ import ( "context" "errors" "fmt" + "maps" + "slices" "sort" "strings" "sync" @@ -79,6 +81,9 @@ type Checker struct { } // Bound is how long one check may run; none when 0 (DefaultBound). Bound time.Duration + // Rebuilds hears the builder's build keys a failed verdict names (failedBuilds): once each is written done, the check of + // those heads is asked again by the checker itself (ADR 0603 §10), as a failure of the nodes is; nil asks none again. + Rebuilds BuildEnds // Asks keeps the asks each running check put, by verdict key (AsksState), for checker_ask_again; nil keeps none. Asks AsksKV // Link is the base of the address a status points at; the verdict key follows. @@ -104,6 +109,8 @@ type Checker struct { held map[string]string // by verdict key: the turn that holds its check off, as last said live map[string]liveCheck // by verdict key: the check running in this process, ended when its heads go settling map[string]*window // by candidates key: the window its heads settle in + rebuilt map[string]bool // by verdict key: every failed build its verdict names is done again + awaiting map[string]bool // by verdict key: its failed builds are heard (awaitRebuilt) // outsideSaid are the candidates outside the mesh said in the log, by group and heads. outsideSaid map[string]bool } @@ -162,6 +169,7 @@ func (ch *Checker) init() { ch.known, ch.inflight, ch.again, ch.tried = map[string]delivery.Candidate{}, map[string]bool{}, map[string]bool{}, map[string]bool{} ch.nodesFail, ch.held = map[string]uint64{}, map[string]string{} ch.live, ch.settling = map[string]liveCheck{}, map[string]*window{} + ch.rebuilt, ch.awaiting = map[string]bool{}, map[string]bool{} ch.outsideSaid = map[string]bool{} }) } @@ -380,8 +388,9 @@ func (ch *Checker) consider(ctx context.Context, key string, c delivery.Candidat // Every check is asked at once: the build seats' streams hold the backlog, a new head's check and a check after a // failure of the nodes on check, a re-judge after a trunk move on recheck, pulled after them (ADR 0525 §4, §9; // ADR 0536 §4). The checker holds no place and no capacity of its own. - ch.prepare(ctx, cs) + ch.prepare(ctx, key, cs) ch.try(ctx, run, key, cs) + ch.hearFailedBuilds(ctx, key, cs) // a check it failed on a build is asked again once that build is done again ch.mu.Lock() more := ch.again[vkey] delete(ch.again, vkey) @@ -518,7 +527,7 @@ const ( // or sibling commits that moved since is carried, its packages run only where the move changed their digest, and a pass's // commits are named as the carry's CarriedFrom. A verdict that stands is ranked with the carries: its claim finds nothing // to do. -func (ch *Checker) prepare(ctx context.Context, cs *Case) { +func (ch *Checker) prepare(ctx context.Context, group string, cs *Case) { cs.Kind, cs.CarriedFrom, cs.Composition = KindNew, nil, nil value, _, found, err := ch.Verdicts.Get(ctx, cs.Key) if err != nil || !found { @@ -531,6 +540,10 @@ func (ch *Checker) prepare(ctx context.Context, cs *Case) { case IsNodesFailure(v): cs.Kind = KindNodes return + case failedBuilds(v, cs) != nil: + cs.Kind = KindNodes // asked again on check once its builds are done again (awaitRebuilt), never by a recheck + ch.awaitRebuilt(ctx, group, cs.Key, failedBuilds(v, cs)) + return case IsConflictFailure(v): return // a head git refused: its next check is whole (ADR 0548 §7) case IsEnded(v), IsUntrunked(v): @@ -687,6 +700,8 @@ func (ch *Checker) claim(ctx context.Context, cs *Case, first bool, epoch uint64 return nil, nil case IsEnded(cur), ch.stale(cur, cs): retake = true // heads held again after their check was ended unjudged (issue 1890), or a trunk that moved + case cur.State == delivery.VerdictFail && ch.takeRebuilt(cs.Key): + retake = true // every failed build it names is done again (ADR 0603 §10) case cur.State == delivery.VerdictFail && IsNodesFailure(cur): ch.mu.Lock() at, ok := ch.nodesFail[cs.Key] @@ -1070,3 +1085,61 @@ func (ch *Checker) movedSince(cs *Case) bool { } return false } + +// awaitRebuilt hears, once per verdict key, the builder's keys of the builds a failed verdict names: when each is written +// done again (builder_build_again), the check of those heads is asked again (claim retakes it), as a failure of the nodes +// is after the nodes changed, so no recheck by hand is needed (ADR 0603 §10). Heard on the builder's own writes, never +// polled; it ends with the checker. +func (ch *Checker) awaitRebuilt(ctx context.Context, group, vkey string, builds map[string]string) { + if ch.Rebuilds == nil { + return + } + ch.mu.Lock() + if ch.awaiting[vkey] { + ch.mu.Unlock() + return + } + ch.awaiting[vkey] = true + ch.mu.Unlock() + ch.wg.Add(1) + go func() { + defer ch.wg.Done() + done := ch.Rebuilds.Done(ctx, builds) + ch.mu.Lock() + delete(ch.awaiting, vkey) + if done { + ch.rebuilt[vkey] = true + } + c, known := ch.known[group] + ch.mu.Unlock() + if done && known && ctx.Err() == nil { + ch.logf("%s: the builds of %s it failed on are done again, so it is checked again (ADR 0603 §10)", vkey, strings.Join(slices.Sorted(maps.Keys(builds)), ", ")) + ch.consider(ctx, group, c) + } + }() +} + +// hearFailedBuilds starts awaitRebuilt for the case's verdict when it names failed builds (failedBuilds). +func (ch *Checker) hearFailedBuilds(ctx context.Context, group string, cs *Case) { + if ch.Rebuilds == nil || ctx.Err() != nil { + return + } + value, _, found, err := ch.Verdicts.Get(ctx, cs.Key) + if err != nil || !found { + return + } + if v, err := delivery.DecodeVerdict(value); err == nil { + if builds := failedBuilds(v, cs); builds != nil { + ch.awaitRebuilt(ctx, group, cs.Key, builds) + } + } +} + +// takeRebuilt says whether every failed build the verdict of vkey names is done again, and forgets it: one retake each. +func (ch *Checker) takeRebuilt(vkey string) bool { + ch.mu.Lock() + defer ch.mu.Unlock() + was := ch.rebuilt[vkey] + delete(ch.rebuilt, vkey) + return was +} diff --git a/modules/mesh-delivery-checker/internal/check/composition_test.go b/modules/mesh-delivery-checker/internal/check/composition_test.go index 53f42d07..72ec4ecb 100644 --- a/modules/mesh-delivery-checker/internal/check/composition_test.go +++ b/modules/mesh-delivery-checker/internal/check/composition_test.go @@ -125,6 +125,8 @@ func (f *fakeEnds) Ended(_ context.Context, builds map[string]string) (map[strin return out, nil } +func (f *fakeEnds) Done(context.Context, map[string]string) bool { return false } + // knownRuns are the modules the trunk holds a record of (the indexer's modules): any commit. type knownRuns []string @@ -153,7 +155,8 @@ func aChangeToABuildModuleOrBaseNamesTheModulesBuiltOnIt(t *testing.T) { runs knownRuns // nil: the trunk holds every module want []string waited string // the modules waited on, sorted - failed string // the failure's test, when the check fails + failed string // the failure's test, when the check fails on a build + tests bool // the tests fail: the check fails before it would wait }{ {name: "the Go build module changes", manifest: map[string]catalogue.Manifest{"mesh-build-go": {Module: "mesh-build-go", Build: goBuild}, "web": {Module: "web", Build: goBuild}, @@ -175,6 +178,9 @@ func aChangeToABuildModuleOrBaseNamesTheModulesBuiltOnIt(t *testing.T) { {name: "a build fails", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}, "mail": {Module: "mail", Build: goBuild}}, builds: map[string]delivery.BuilderBuild{"web": {State: delivery.BuildFailed, Error: "go build: undefined: x"}, "mail": done}, waited: "mail, web", failed: "build: web"}, + {name: "the tests fail", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}}, + builds: map[string]delivery.BuilderBuild{"web": {State: delivery.BuildBuilding}}, tests: true, + want: []string{"composes at once, without waiting on the builds of web"}}, {name: "a build does not end within the bound", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}}, builds: map[string]delivery.BuilderBuild{"web": {State: delivery.BuildBuilding}}, want: []string{"the builds of web did not end within the build-wait-bound"}, waited: "web"}, @@ -185,6 +191,13 @@ func aChangeToABuildModuleOrBaseNamesTheModulesBuiltOnIt(t *testing.T) { want: []string{"the builds of web cannot be heard"}}, } { b := candidateBench(t, true) + if row.tests { + b.test = func(c stepCall) (any, error) { + r := captured[Tested](t, "go-test-failing") + r.Level, _ = c.args["level"].(string) + return r, nil + } + } comps := &fakeCompositions{v: delivery.CompositionVerdict{Group: "feat/x", Heads: "0123456789ab", State: delivery.CompositionPass, SDK: "x", Machines: []string{"g14"}}} b.ex.Compositions = comps var ends *fakeEnds @@ -216,9 +229,12 @@ func aChangeToABuildModuleOrBaseNamesTheModulesBuiltOnIt(t *testing.T) { } } switch { - case row.failed != "" && (v.State != delivery.VerdictFail || v.Failed.Test != row.failed || !strings.Contains(v.Failed.Words, "undefined: x")): - t.Errorf("%s: the verdict is %s failing %+v, want it failed naming %s and the builder's words", row.name, v.State, v.Failed, row.failed) - case row.failed == "" && v.State != delivery.VerdictPass: + case row.failed != "" && (v.State != delivery.VerdictFail || v.Failed.Test != row.failed || !strings.Contains(v.Failed.Words, "undefined: x") || + !strings.Contains(v.Failed.Words, "the check asks itself again when the build is done")): + t.Errorf("%s: the verdict is %s failing %+v, want it failed naming %s, the builder's words and how it is asked again", row.name, v.State, v.Failed, row.failed) + case row.tests && v.State != delivery.VerdictFail: + t.Errorf("%s: %+v, want the tests' failure", row.name, v) + case row.failed == "" && !row.tests && v.State != delivery.VerdictPass: t.Errorf("%s: %+v", row.name, v) } words := compositionWords(v.Composition, v.Took.Composition) diff --git a/modules/mesh-delivery-checker/internal/check/read.go b/modules/mesh-delivery-checker/internal/check/read.go index 2086eced..1fc0efbe 100644 --- a/modules/mesh-delivery-checker/internal/check/read.go +++ b/modules/mesh-delivery-checker/internal/check/read.go @@ -350,7 +350,7 @@ type Status struct { Lost map[string]string `json:"lost"` // BuildsNotEnded are, of each candidate whose check runs, the builder's builds of its changed modules that are neither // done nor failed, by group: ` `, sorted (no key yet is `not begun`). A check waits on them before it - // composes (ADR 0603 §10), but for the modules its line names as composed without them; builder_build_again builds a failed one again. + // composes (ADR 0603 §10), but for the modules its line names as composed without them; builder_build_again builds a failed one again, and the check asks itself again when it is done. BuildsNotEnded map[string]string `json:"builds_not_ended,omitempty"` // DeadLetters says whether the checker hears the asks the build seats give up on (issue 734): deaf, a try or check // whose ask is given up waits until its bound.