Merge pull request 'A pull request's check composes only after the change's own builds end (issue 1921)' (#164) from fix/1921-compose-waits-builds into main

This commit was merged in pull request #164.
This commit is contained in:
2026-11-07 13:28:38 +00:00
4 changed files with 81 additions and 33 deletions
@@ -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
}
}
@@ -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
}
@@ -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 {
@@ -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).
@@ -250,23 +251,53 @@ 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
}
// 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) {
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 {
@@ -358,9 +389,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).