The check hands the composer at a node-engine head the seat list it judges against (issue 1966)

The group try of 2026-11-06 10:07 composed with a composer that had no
seat list, so its composition failed on all four machines for paths and
holders the seats define. The checker's one list per check (the
reader's, the members' heads' laid over it, ADR 0636 §4) now goes to the
judge and the sandbox composer alike; a list the reader has not read
goes to neither, and the composer then judges no machine (ADR 0636 §1).

Claude-Session: https://claude.ai/code/session_01NcbgfFSnn24M3GcqttoU7i
This commit is contained in:
2026-11-06 12:03:05 +01:00
parent c7cfd7fa05
commit a92dc95f94
6 changed files with 80 additions and 12 deletions
@@ -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).
@@ -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
@@ -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
@@ -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.
@@ -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).
@@ -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
}