The checker previews a member whose fetch merged, whatever its later steps ended with
mesh/delivery merged: source.main moved to novox/mesh-delivery@a5d710863157
mesh/delivery-group group fix/checker-preview-after-failed-step merged: every member is merged
mesh/check checking 24d9c6cdf8d9 on a5d710863157 | novox/mesh-delivery#208 in group fix/checker-preview-after-failed-step

A member whose check step errored after its fetch merged was left unpreviewed,
so one failing level hid the manifest validator and the composition from the
check, and the words "its head does not merge or its fetch did not answer"
were wrong for it. The checker now reads the fetch a failed step answers
beside its error and previews the merge from it; only a head that does not
merge or a fetch that did not answer stays unpreviewed, and the check says
which of the two.

Builds: design 47 G A-286 (G-22-lead-answers-2.md at hq 41dd30fe; issue 1921; principle 3)
Quick: this checker on main; thorough: the same rule in G's checker (join 11, J15).

Claude-Session: https://claude.ai/code/session_01NcbgfGKaxw984XyAPfLm4h
This commit is contained in:
2026-11-12 15:25:44 +01:00
parent 76db40eb6b
commit 24d9c6cdf8
2 changed files with 43 additions and 11 deletions
@@ -245,11 +245,12 @@ func (b *bench) answer(a buildstep.StepAsk) buildstep.Step {
}
out, err := gobuild.RunAsk(context.Background(), steps, a)
step := buildstep.Step{ID: a.ID, Tool: a.Kind + " ask", Node: node, State: buildstep.StepPassed, Started: began, Ended: began.Add(2 * time.Second)}
if out != nil {
step.Result, _ = json.Marshal(out) // a failed step keeps what it answered beside its error, as the step runner does
}
if err != nil {
step.State, step.Reason = buildstep.StepFailed, err.Error()
return step
}
step.Result, _ = json.Marshal(out)
return step
}
@@ -589,9 +590,11 @@ func TestANodePackageIsNeverKeptAndRunsEveryCheck(t *testing.T) { // 1288/AC4, 1
// change's own tests still run first (issue 1289), and the rest of the level after them. A member that only failed its
// tests still gets its preview, so the composition is read and its failure named after the tests'. A member whose head
// does not merge has no preview of a merge, so the assigner's composition would only wait: the check does not wait for it
// and says why it was not run (review of 650a1d0).
// and says why it was not run (review of 650a1d0). A member whose step failed after its fetch merged is previewed from the
// fetch the failed step answered, and the composition is read (design 47 G A-286).
func TestEveryFailureOfEveryMemberLevelAndTheCompositionIsNamed(t *testing.T) {
for _, conflicts := range []bool{false, true} {
for _, kind := range []string{"tests", "conflicts", "errors"} {
conflicts, errs := kind == "conflicts", kind == "errors"
b := newBench(t)
comps := &fakeCompositions{v: refusedOn("x", "g14", "route-proxy is defined twice"), waits: conflicts}
b.ex.Compositions = comps
@@ -606,6 +609,9 @@ func TestEveryFailureOfEveryMemberLevelAndTheCompositionIsNamed(t *testing.T) {
}
failing := captured[Tested](t, "go-test-failing")
b.test = func(c stepCall) (any, error) {
if errs && c.args["level"] == "unit" {
return nil, errors.New("the seat definitions in . do not generate")
}
if got := packagesOf(c); c.args["level"] == "integration" || (len(got) == 1 && got[0] == "m/internal/b") {
r := failing
r.Level = c.args["level"].(string)
@@ -622,6 +628,14 @@ func TestEveryFailureOfEveryMemberLevelAndTheCompositionIsNamed(t *testing.T) {
case <-time.After(5 * time.Second):
t.Fatalf("conflicts %v: the check waits on a composition that only waits", conflicts)
}
if errs {
keys, _ := b.prev.Keys(context.Background())
if len(keys) != 2 || len(comps.asked) == 0 || v.Composition == nil || v.Composition.State == meshdeliverycheckerclient.CompositionNotRun {
t.Errorf("a step failed after its fetch merged: previews %v, asked %v, composition %+v, want both previews and the composition read",
keys, comps.asked, v.Composition)
}
continue
}
var got []string
for _, f := range v.Failures {
got = append(got, firstFailed(meshdeliverychecker.VerdictsByGroupHeadsValue{Failed: &FailedFirst{Test: f.Test, Machine: f.Machine, Words: f.Words}}))
@@ -644,7 +658,7 @@ func TestEveryFailureOfEveryMemberLevelAndTheCompositionIsNamed(t *testing.T) {
case !conflicts && (len(keys) != 2 || v.Composition == nil || v.Composition.State != meshdeliverycheckerclient.CompositionFail):
t.Errorf("tests alone failed: previews %v, composition %+v, want both previews and the composition read", keys, v.Composition)
case conflicts && (len(comps.asked) != 0 || v.Composition == nil || v.Composition.State != meshdeliverycheckerclient.CompositionNotRun ||
!strings.Contains(v.Composition.Words, "novox/mesh-sdk#12")):
!strings.Contains(v.Composition.Words, "novox/mesh-sdk#12 (its head does not merge)")):
t.Errorf("a member did not merge: asked %v, composition %+v, want not run naming novox/mesh-sdk#12", comps.asked, v.Composition)
}
var unit []string
@@ -284,11 +284,14 @@ type state struct {
judgeHead string
// predefinedHead is the catalogue member's head when its merge changes predefined.json: the list is read there.
predefinedHead string
// 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 are the members (pullKey) with no preview of a merge, each with which of the two it is: a head that does not
// merge, or a fetch that did not answer (design 47 G A-286). The indexer holds no manifest of them, so the assigner's
// composition would only wait: it is not awaited. A member whose fetch merged is previewed whatever its later steps ended with.
unpreviewed []string
// tags are the group's tagged versions its members are built against (ADR 0607 §3), once the check heard them.
tags groupVersions
// conflicted are the members (pullKey) whose head does not merge into its trunk.
conflicted map[string]bool
// outOfStep are, by pull request (pullKey), the generated files a level's compare step found out of step (design 26).
outOfStep map[string][]string
// noCandidate says why the case has no previewed candidate of its heads from the indexer within the bound: for a whole
@@ -344,7 +347,7 @@ func (st *state) takeTryCandidate(ctx context.Context) {
return
}
if len(st.unpreviewed) > 0 {
no("no preview of a merge of " + strings.Join(st.unpreviewed, ", ") + " (its head does not merge or its fetch did not answer)")
no("no preview of a merge of " + strings.Join(st.unpreviewed, ", "))
return
}
if st.c.Wait != nil {
@@ -425,7 +428,7 @@ func (st *state) compose(ctx context.Context) {
if len(st.unpreviewed) > 0 {
// An engine would only say it cannot read a manifest the indexer will not have: the check does not ask it.
st.composition = &rules.Composition{NotRun: "no preview of a merge of " + strings.Join(st.unpreviewed, ", ") +
" (its head does not merge or its fetch did not answer), so the change cannot be composed"}
", so the change cannot be composed"}
return
}
// ADR 0603 §10: the check composes with the candidate's builds; whether it failed already is the running rules' word
@@ -593,13 +596,16 @@ func (st *state) judgeAll(ctx context.Context) {
st.ask(ctx, t, &merged)
}
// Every build module's fetch merged (each is in merged once): the preview is written, also when the member's tests
// failed, so the indexer holds its manifests and the assigner can compose the change (review of 650a1d0).
// or a later step failed, so the indexer holds its manifests and the assigner can compose the change (review of
// 650a1d0; design 47 G A-286).
switch {
case st.narrowTry(): // a try of a level or some tests owes the indexer no preview: it judges no manifest
case len(merged) == len(modules):
st.j.writePreview(ctx, p, merged)
case st.conflicted[pullKey(p)]:
st.unpreviewed = append(st.unpreviewed, pullKey(p)+" (its head does not merge)")
default:
st.unpreviewed = append(st.unpreviewed, pullKey(p))
st.unpreviewed = append(st.unpreviewed, pullKey(p)+" (its fetch did not answer)")
}
st.judgedByMember(p, merged)
// The old form (ADR 0500 §5): the group's package member packed by the Node fetch, kept only while no tagged
@@ -730,6 +736,14 @@ func (st *state) askNeeding(ctx context.Context, t *target, merged *[]Fetched, l
st.facts = append(st.facts, rules.Step{Repository: p.Repository, Number: p.Number, Module: t.module, Node: step.Node})
st.c.Progress.Set(fmt.Sprintf("judge the check of %s#%d", p.Repository, p.Number), step.Node)
if step.State != buildstep.StepPassed {
// A step that failed after its fetch merged answers the fetch beside its error (design 47 G A-286): the merge is
// previewed from it, so its manifests are judged and the change composed whatever the step ended with.
var failed buildstep.CheckAnswer
if !placed && json.Unmarshal(step.Result, &failed) == nil && failed.Fetch.Workspace != "" && len(failed.Fetch.Conflicts) == 0 {
t.fetched, t.changed = failed.Fetch, failed.Fetch.Changed
*merged = append(*merged, t.fetched)
st.checkBinariesOf(t)
}
st.fail(step.Node, fmt.Sprintf("the check of %s#%d %s: %s", p.Repository, p.Number, step.State, step.Reason))
return
}
@@ -743,6 +757,10 @@ func (st *state) askNeeding(ctx context.Context, t *target, merged *[]Fetched, l
return
}
if len(t.fetched.Conflicts) > 0 {
if st.conflicted == nil {
st.conflicted = map[string]bool{}
}
st.conflicted[pullKey(p)] = true
if !st.narrowTry() {
st.j.writePreview(ctx, p, append(*merged, t.fetched))
}