diff --git a/modules/mesh-delivery-checker/internal/check/case.go b/modules/mesh-delivery-checker/internal/check/case.go index cb60980f..882fafaf 100644 --- a/modules/mesh-delivery-checker/internal/check/case.go +++ b/modules/mesh-delivery-checker/internal/check/case.go @@ -1,6 +1,7 @@ package check import ( + "encoding/json" "fmt" "maps" "slices" @@ -45,6 +46,9 @@ type Case struct { Queued time.Time // Try narrows a try's check (its level, its tests); nil for a pull request's. Try *TryOptions + // Seats is the seat list the check composes against (state.checkSeats), raw, handed to the composer of a node-engine + // member's head; nil when the checker's reader has not read its list. + Seats []json.RawMessage notesMu sync.Mutex // warnings are, by pull request, what part of its check did not run (Case.warn). diff --git a/modules/mesh-delivery-checker/internal/check/composition_test.go b/modules/mesh-delivery-checker/internal/check/composition_test.go index 20a810a0..bafc264a 100644 --- a/modules/mesh-delivery-checker/internal/check/composition_test.go +++ b/modules/mesh-delivery-checker/internal/check/composition_test.go @@ -2,6 +2,7 @@ package check import ( "context" + "encoding/json" "errors" "maps" "slices" @@ -22,12 +23,15 @@ type fakeCompositions struct { v meshwants.CompositionsByGroupHeadsValue err error asked []string + // seats are the seat lists the cases asked carried (Case.Seats), one per ask. + seats [][]json.RawMessage // waits answers only when its context ends. waits bool } func (f *fakeCompositions) Compose(ctx context.Context, c *Case, _ buildstep.PackageVersions) (meshwants.CompositionsByGroupHeadsValue, error) { f.asked = append(f.asked, c.Key) + f.seats = append(f.seats, c.Seats) if f.waits { <-ctx.Done() return meshwants.CompositionsByGroupHeadsValue{}, ctx.Err() @@ -57,6 +61,26 @@ func TestAPassingCheckTakesTheAssignersComposition(t *testing.T) { if v.Composition.Fpset == "" || v.Composition.Fpset != cs.FPSet { t.Errorf("the composition names the fingerprint set %q, want the candidate's %q (ADR 0548 §5)", v.Composition.Fpset, cs.FPSet) } + // Issue 1966 (the group try of 2026-11-06 10:07): the composition is handed the check's seat list, the judge's, once the + // checker's reader read it; a list not read hands none (the composer then judges no machine, ADR 0636 §1). + if len(f.seats) != 1 || f.seats[0] != nil { + t.Errorf("with no seat list reader the composition was handed %v, want none", f.seats) + } + for _, read := range []bool{true, false} { + b.ex.SeatList = fakeSeatList{read: read} + b.judge(t, caseOf(t, pullOf(repoA, 7, head1))) + got := f.seats[len(f.seats)-1] + if want := 0; read { + want = 2 + if len(got) != want || string(got[0]) != `{"seat":"a"}` { + t.Errorf("with the list read the composition was handed %s, want the reader's two definitions", got) + } + } else if len(got) != want { + t.Errorf("with the list not read the composition was handed %s, want none", got) + } + } + b.ex.SeatList = nil + f.asked, f.seats = f.asked[:1], f.seats[:1] // ADR 0548 §5: a carry whose fingerprints did not change keeps the composition it passed and asks the assigner nothing. carry := caseOf(t, pullOf(repoA, 7, head1)) carry.CarriedFrom, carry.Composition = map[string]string{repoA: trunkA2}, v.Composition diff --git a/modules/mesh-delivery-checker/internal/check/engines.go b/modules/mesh-delivery-checker/internal/check/engines.go index a4b0bb1c..600556df 100644 --- a/modules/mesh-delivery-checker/internal/check/engines.go +++ b/modules/mesh-delivery-checker/internal/check/engines.go @@ -237,7 +237,7 @@ func (e EngineCompositions) inSandbox(ctx context.Context, c *Case, tagged build return a, fmt.Errorf("the engine of %s answered a composition input that cannot be encoded again: %w", node, err) } req := composer.Request{V: composer.Version, Input: input, Depth: composer.DepthCompose, Machines: []string{node}, - Cases: []composer.Case{{Name: "change", Manifests: manifests, Verdicts: verdicts}}} + Cases: []composer.Case{{Name: "change", Manifests: manifests, Verdicts: verdicts}}, Seats: c.Seats} raw, err := json.Marshal(req) if err != nil { return a, err diff --git a/modules/mesh-delivery-checker/internal/check/engines_test.go b/modules/mesh-delivery-checker/internal/check/engines_test.go index 1587dab0..5f41d060 100644 --- a/modules/mesh-delivery-checker/internal/check/engines_test.go +++ b/modules/mesh-delivery-checker/internal/check/engines_test.go @@ -152,6 +152,8 @@ func TestTheCompositionIsMadeFromTheEnginesAnswers(t *testing.T) { host := CaseOf(c, map[string]string{repoA: trunkA, nodeEngineRepo: trunkA}) withInput := composed("anchor") withInput.Input = map[string]any{"nodes": []any{map[string]any{"name": "anchor"}}} + // Issue 1966 (the group try of 2026-11-06 10:07): the composer at the head composes against the check's seat list. + host.Seats = []json.RawMessage{json.RawMessage(`{"format":1,"seat":"node-uplink"}`)} e := &fakeEngines{answers: map[string]ne.ComposeCandidateAnswer{"anchor": withInput}} sb := &fakeSandbox{v: func(node string) composer.Verdict { return composer.Verdict{V: composer.Version, Cases: []composer.CaseVerdict{{Name: "change", @@ -170,7 +172,8 @@ func TestTheCompositionIsMadeFromTheEnginesAnswers(t *testing.T) { var req composer.Request _ = json.Unmarshal(job.Request, &req) if job.Fetch.Repository != nodeEngineRepo || job.Fetch.Commit != head2 || job.Package != composer.Package || - string(req.Input) != `{"nodes":[{"name":"anchor"}]}` || req.Cases[0].Manifests["web"].Digest == "" { + string(req.Input) != `{"nodes":[{"name":"anchor"}]}` || req.Cases[0].Manifests["web"].Digest == "" || + len(req.Seats) != 1 || string(req.Seats[0]) != `{"format":1,"seat":"node-uplink"}` { t.Errorf("job = %+v, request %+v", job, req) } // Issue 1921: the composer is built at the group's heads, also a member that publishes no package yet. diff --git a/modules/mesh-delivery-checker/internal/check/judge.go b/modules/mesh-delivery-checker/internal/check/judge.go index c4326d2e..f138bddc 100644 --- a/modules/mesh-delivery-checker/internal/check/judge.go +++ b/modules/mesh-delivery-checker/internal/check/judge.go @@ -304,6 +304,9 @@ type state struct { // 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 string + // seats is the check's seat list once read (checkSeats), and seatsErr why it is not. + seats *checkSeats + seatsErr error } // narrowTry says the case is a try narrowed to a level or to some tests: it judges no manifest, graph or composition. @@ -400,6 +403,14 @@ func (st *state) composed(ctx context.Context, v *meshdeliverychecker.VerdictsBy return } words := st.awaitBuilds(ctx, v) // ADR 0603 §10: the check composes with the candidate's builds + // The composer of a node-engine member's head composes against the check's seat list, the judge's (ADR 0635 §1, ADR 0636 + // §4); only a list the reader read is handed: the heads' alone are no list, and the composer then judges no machine. + st.c.Seats = nil + if seats, err := st.checkSeats(ctx); err != nil { + st.c.note("the composition is given no seat list: " + err.Error()) + } else if seats.listed { + st.c.Seats = seats.defs + } comp, err := st.j.Compositions.Compose(ctx, st.c, st.tags.goPackages) if err != nil { // Said in the verdict, never left as a composition not yet run (review finding 4). diff --git a/modules/mesh-delivery-checker/internal/check/manifests_judge.go b/modules/mesh-delivery-checker/internal/check/manifests_judge.go index da9c5e6d..0ec6cba7 100644 --- a/modules/mesh-delivery-checker/internal/check/manifests_judge.go +++ b/modules/mesh-delivery-checker/internal/check/manifests_judge.go @@ -132,19 +132,11 @@ func (st *state) judgeManifests(ctx context.Context) *meshdeliverychecker.Verdic st.c.note("the manifests are judged against the predefined keys of " + from) } req.Packages = st.knownPackages(ctx) - if st.j.SeatList != nil { - seats, err := st.j.SeatList.Definitions() - if err != nil { - return fail(goModule, "the change's manifests cannot be judged: the seat list is not read ("+err.Error()+")") - } - req.Seats = seats - } - // The members' heads' definitions over the list's (ADR 0636 §4): the trunk's manifest validator of a new indexer as well. - heads, err := st.headSeats(ctx) + seats, err := st.checkSeats(ctx) if err != nil { return fail(goModule, "the change's manifests cannot be judged: "+err.Error()) } - req.Seats = overSeats(req.Seats, heads) + req.Seats = seats.defs cv, trunkCase, v := st.runJudge(ctx, req, touched, commit, at, st.packagesOf(DeliveryRepository)) if v != nil { return v @@ -435,3 +427,37 @@ func (st *state) knownPackages(ctx context.Context) []string { func (st *state) trunkJudgeVersions() buildstep.PackageVersions { return packagesFor(st.j.Published, &Case{}, nil, DeliveryRepository) } + +// checkSeats is the seat list a check judges and composes against, read once per check: the list the checker's reader holds +// with the members' heads' definitions laid over it (ADR 0636 §4), the trunk's manifest validator of a new indexer as well. +// listed says the reader's list was read; without a reader it is the heads' alone. An error is a list that is not read or a +// head whose definitions are not taken. +func (st *state) checkSeats(ctx context.Context) (checkSeats, error) { + if st.seats != nil { + return *st.seats, st.seatsErr + } + var out checkSeats + err := func() error { + if st.j.SeatList != nil { + defs, err := st.j.SeatList.Definitions() + if err != nil { + return fmt.Errorf("the seat list is not read (%s)", err.Error()) + } + out.defs, out.listed = defs, true + } + heads, err := st.headSeats(ctx) + if err != nil { + return err + } + out.defs = overSeats(out.defs, heads) + return nil + }() + st.seats, st.seatsErr = &out, err + return out, err +} + +// checkSeats is a check's seat list (state.checkSeats): the raw definitions, and whether the reader's list is among them. +type checkSeats struct { + defs []json.RawMessage + listed bool +}