From 3f9acf0223259b709cae5018f38fb05bc37dc6a8 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 7 Nov 2026 12:48:11 +0100 Subject: [PATCH 1/2] A check composes only after the candidate's builds it waits on have ended (issue 1921) The indexer writes a pull request's candidate first from its pull message alone, every module's fingerprint "no merge preview is computed", and the fingerprints only with the preview, on the same heads. A check that began on that first write kept its case: it waited on no build, said nothing, and the engines, reading the previewed candidate, refused the module as not yet built (mesh-catalog #624, mesh-delivery #163). Such a check now takes the previewed candidate before it judges the manifests, waits on the builds and composes, as a whole try already does. A build not ended within the build-wait-bound is named and the change is not composed, never an engine's refusal of a fingerprint it has no done build of read as the change's. Claude-Session: https://claude.ai/code/session_01NcbgfFSnn24M3GcqttoU7i --- .../mesh-delivery-checker/try_candidates.go | 9 ++-- .../internal/check/builds.go | 28 ++++++------ .../internal/check/composition_test.go | 22 ++++++++-- .../internal/check/judge.go | 43 ++++++++++++++++--- 4 files changed, 72 insertions(+), 30 deletions(-) diff --git a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/try_candidates.go b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/try_candidates.go index 2eeb9a08..e00013b9 100644 --- a/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/try_candidates.go +++ b/modules/mesh-delivery-checker/cmd/mesh-delivery-checker/try_candidates.go @@ -8,12 +8,9 @@ import ( "github.com/nats-io/nats.go/jetstream" "git.novox.be/novox/mesh-sdk/go/delivery" -) -// candidateWaitsOnPreview is the None2 of a module the indexer wrote from a pull message alone, before it had the merge -// preview of the head (the indexer's NoneNoPreview): its fingerprint comes with the preview, so a candidate that still says -// it is not yet the candidate of the heads previewed. It moves to mesh-sdk's delivery package with the next change there. -const candidateWaitsOnPreview = "no merge preview is computed" + "mesh-delivery-checker/internal/check" +) // tryCandidates answers the indexer's candidate of a try's group (novox/hq ADR 0593 §7, issue 1921): the candidate the // engines compose and the manifest validator judges, which the indexer makes of the try's previews. It waits for it on the @@ -63,7 +60,7 @@ func (t tryCandidates) TryCandidate(ctx context.Context, group, heads string, bo func waitsOnPreview(c delivery.Candidate) bool { for _, p := range c.Pulls { for _, m := range p.Fps { - if m.None2 == candidateWaitsOnPreview { + if check.WaitsOnPreview(m) { return true } } diff --git a/modules/mesh-delivery-checker/internal/check/builds.go b/modules/mesh-delivery-checker/internal/check/builds.go index a35b51ec..5a7473af 100644 --- a/modules/mesh-delivery-checker/internal/check/builds.go +++ b/modules/mesh-delivery-checker/internal/check/builds.go @@ -122,21 +122,22 @@ func (j *Examiner) buildWait() time.Duration { // 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) and the failures of the builds that failed, for -// the rules to take in (rules.JudgeComposition); failed says the check has failed already. -func (st *state) awaitBuilds(ctx context.Context, failed bool) ([]string, []delivery.FailedFirst) { +// a build not ended within the bound is named in late, and the check does not compose: an engine composes a module only at +// a done build, so its answer would be the builder's lateness read as the change's refusal (issue 1921). It answers the +// words of the composition's warnings (what the check composes without waiting on its build, and why) and the failures of +// the builds that failed, for the rules to take in (rules.JudgeComposition); failed says the check has failed already. +func (st *state) awaitBuilds(ctx context.Context, failed bool) (words []string, builds []delivery.FailedFirst, late string) { wait, words := st.buildsOfTheCheck(ctx) if len(wait) == 0 { - return words, nil + return words, nil, "" } modules := slices.Sorted(maps.Keys(wait)) if failed { // 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, ", "))), nil + 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, ", "))), nil, "" } 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, ", "))), 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, ", "))), nil, "" } 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) @@ -147,21 +148,20 @@ func (st *state) awaitBuilds(ctx context.Context, failed bool) ([]string, []deli 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)), nil + 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)), nil, "" } - var late []string - var builds []delivery.FailedFirst + var notEnded []string for _, module := range modules { b, done := ended[module] switch { case !done: - late = append(late, module) + notEnded = append(notEnded, module) case b.State == delivery.BuildFailed: builds = append(builds, 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 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)) + if len(notEnded) > 0 && ctx.Err() == nil { + late = fmt.Sprintf("the builder's builds of %s did not end within the build-wait-bound of %s, so the change is not composed: an engine composes a module only at a done build (ADR 0603 §10)", strings.Join(notEnded, ", "), bound) } - return words, builds + return words, builds, late } diff --git a/modules/mesh-delivery-checker/internal/check/composition_test.go b/modules/mesh-delivery-checker/internal/check/composition_test.go index e6ae1535..977c8f32 100644 --- a/modules/mesh-delivery-checker/internal/check/composition_test.go +++ b/modules/mesh-delivery-checker/internal/check/composition_test.go @@ -180,6 +180,8 @@ func aChangeToABuildModuleOrBaseNamesTheModulesBuiltOnIt(t *testing.T) { 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 served []string // what the running builds serve (ADR 0614 §7); nil reads nothing of what runs + preview bool // the check began on the candidate the indexer wrote before its merge preview (issue 1921) + late bool // a build did not end within the bound: the change is not composed (issue 1921) }{ {name: "the Go build module invokes a verb no running build serves", manifest: map[string]catalogue.Manifest{"mesh-build-go": {Module: "mesh-build-go", Build: goBuild, @@ -211,8 +213,10 @@ func aChangeToABuildModuleOrBaseNamesTheModulesBuiltOnIt(t *testing.T) { 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"}, + builds: map[string]delivery.BuilderBuild{"web": {State: delivery.BuildBuilding}}, late: true, + want: []string{"the builder's builds of web did not end within the build-wait-bound", "the change is not composed"}, waited: "web"}, + {name: "the check began before the merge preview", manifest: map[string]catalogue.Manifest{"web": {Module: "web", Build: goBuild}}, + builds: map[string]delivery.BuilderBuild{"web": done}, preview: true, 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"}, @@ -251,8 +255,20 @@ func aChangeToABuildModuleOrBaseNamesTheModulesBuiltOnIt(t *testing.T) { b.ex.Read = read p := pullOf(catalogueRepository, 9, head1) p.Fps, p.Modules = fps, slices.Sorted(maps.Keys(fps)) + if row.preview { + previewed := p + b.ex.Candidates = &fakeTryCandidates{cand: delivery.Candidate{FPSet: "fps", Pulls: []delivery.CandidatePull{previewed}}} + p.Fps = map[string]delivery.CandidateModule{} + for module, m := range fps { + p.Fps[module] = delivery.CandidateModule{Manifest: m.Manifest, None2: noPreview} + } + } v := b.judge(t, caseOf(t, p)).Verdict - if v.Composition == nil || len(comps.asked) != 1 { + asks := 1 + if row.late { + asks = 0 // never asked: an engine would read the builder's lateness as the change's refusal + } + if v.Composition == nil || len(comps.asked) != asks || row.late && v.Composition.State != delivery.CompositionNotRun { t.Fatalf("%s: composed %d times: %+v", row.name, len(comps.asked), v) } if ends != nil { diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index bc063e33..cd165a44 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -135,8 +135,9 @@ type Examiner struct { Names(ctx context.Context) ([]string, error) } // Candidates answers the indexer's candidate of a try's group once it wrote it from the try's previews, the manifests - // and fingerprints a group try's manifest validator and composition judge (issue 1921); nil leaves a whole try with no - // manifest or composition judged, said. + // and fingerprints a group try's manifest validator and composition judge (issue 1921), and of a pull request's check + // that began before the indexer read its merge preview; nil leaves such a case with no manifest or composition judged, + // said. Candidates TryCandidates // Inputs keeps the checking program's inputs (the state InputsStateName, served on InputVerb); BeginBound is how long a run ask may wait for a // build node to begin it (DefaultBeginBound when zero). @@ -258,11 +259,35 @@ type state struct { // narrowTry says the case is a try narrowed to a level or to some tests: it judges no manifest, graph or composition. func (st *state) narrowTry() bool { return st.c.Try != nil && !st.c.Try.Whole() } -// takeTryCandidate gives a whole try's pulls the fingerprints and manifests the indexer's candidate of its group holds -// (the group's pull-request check has them from its candidate, CaseOf), once the indexer wrote it from the try's previews. -// A try with no such candidate is said so, in a note and in its composition, and is no pass (Tryer.run). +// noPreview is the None2 of a module the indexer wrote from a pull message alone, before it had the merge preview of the +// head (the indexer's NoneNoPreview): its fingerprint comes with the preview, on the same heads. +const noPreview = "no merge preview is computed" + +// WaitsOnPreview says a candidate's module has no fingerprint yet because the indexer has not read its merge preview: a +// candidate that holds one is not yet the candidate of its heads previewed. +func WaitsOnPreview(m delivery.CandidateModule) bool { return m.None2 == noPreview } + +// previewPending says a module of the case's pulls waits on its merge preview (WaitsOnPreview). +func (st *state) previewPending() bool { + for _, p := range st.c.Pulls { + for _, m := range p.Fps { + if WaitsOnPreview(m) { + return true + } + } + } + return false +} + +// takeTryCandidate gives a whole try's pulls the fingerprints and manifests the indexer's candidate of its group holds, +// once the indexer wrote it from the try's previews. A pull request's check has them from its candidate (CaseOf), but the +// indexer writes a pull request's candidate first from its pull message alone and its fingerprints only with the merge +// preview, on the same heads: a check that began on that first write takes the previewed candidate too, before it judges +// the manifests, waits on the builds and composes (ADR 0603 §10; issue 1921: a check otherwise waited on no build and +// the engines, reading the previewed candidate, refused its modules as not yet built). A case with no such candidate is +// said so, in a note and in its composition, and is no pass (Tryer.run). func (st *state) takeTryCandidate(ctx context.Context) { - if st.c.Try == nil || !st.c.Try.Whole() { + if st.c.Try != nil && !st.c.Try.Whole() || st.c.Try == nil && !st.previewPending() { return } no := func(why string) { @@ -358,9 +383,13 @@ func (st *state) compose(ctx context.Context) { // ADR 0603 §10: the check composes with the candidate's builds; whether it failed already is the running rules' word // on the facts so far, an asking choice only (the verdict is the rules' of every fact). failed := rules.Judge(st.factsOf()).State == delivery.VerdictFail - words, builds := st.awaitBuilds(ctx, failed) + words, builds, late := st.awaitBuilds(ctx, failed) c := &rules.Composition{Waits: words, BuildsFailed: builds, FPSet: st.c.FPSet} st.composition = c + if late != "" { + c.NotRun = late // the builder's lateness, said by name, never the change's refusal (issue 1921) + return + } 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). From c0da09d81f6e30c4135848811c9b7ca70ab11fa9 Mon Sep 17 00:00:00 2001 From: jochen Date: Sat, 7 Nov 2026 14:21:15 +0100 Subject: [PATCH 2/2] A pull request's check without a previewed candidate says its composition was not judged, not its manifests (issue 1921, review note) Claude-Session: https://claude.ai/code/session_01NcbgfFSnn24M3GcqttoU7i --- .../mesh-delivery-checker/internal/check/judge.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index cd165a44..8ea27c1b 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -251,8 +251,10 @@ type state struct { // unpreviewed are the members (pullKey) with no preview of a merge: a head that does not merge, or a fetch that did not // answer. The indexer holds no manifest of them, so the assigner's composition would only wait: it is not awaited. unpreviewed []string - // noCandidate says why a whole try has no candidate of its group from the indexer, so its manifests were not judged and - // it was not composed; empty when it has one, and for a pull request's check. + // noCandidate says why the case has no previewed candidate of its heads from the indexer within the bound: for a whole + // try its manifests were not judged and it was not composed; for a pull request's check that began on the indexer's + // first write, its manifests were judged from that write and it was not composed. Empty when it has one, and for a pull + // request's check whose case already holds its fingerprints. noCandidate string } @@ -291,7 +293,11 @@ func (st *state) takeTryCandidate(ctx context.Context) { return } no := func(why string) { - st.noCandidate = "the group's manifests and composition were not judged: " + why + what := "the group's manifests and composition were not judged: " + if st.c.Try == nil { + what = "the composition was not judged (the manifests were judged from the indexer's first write): " + } + st.noCandidate = what + why st.c.note(st.noCandidate) } if st.j.Candidates == nil {