mesh-delivery-checker: a placement that cannot be read leaves the composition not made, said

Review finding 4: an unreadable placement gave no nodes, so nobody was asked and the check passed. The
placement read now answers its error, and the verdict says the composition was not made and why.

Claude-Session: https://claude.ai/code/session_01NcbgfJ9Gum47zkFKWRaovL

modules/mesh-delivery-checker/cmd/mesh-delivery-checker/engines.go

@@ -3,6 +3,7 @@ package main
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"sort"
 	"sync"
@@ -51,24 +52,25 @@ func (e engineAsks) ComposeCandidate(ctx context.Context, node string, ask deliv
 }
 
 // placedNodes names every node the seat mesh-placement places now, read from its state when a check composes (no
-// watch: a check reads it once). A state that cannot be read places no node, said in the journal.
+// watch: a check reads it once). A state that cannot be read is an error, so the check says its composition was not
+// made rather than passing with no node asked (review finding 4).
 type placedNodes struct {
-	kv   func(ctx context.Context) (jetstream.KeyValue, error)
-	logf func(string, ...any)
+	kv func(ctx context.Context) (jetstream.KeyValue, error)
 }
 
-func (p placedNodes) Placed() []string {
+func (p placedNodes) Placed() ([]string, error) {
 	ctx, cancel := context.WithTimeout(context.Background(), delivery.ComposeAnswerBound)
 	defer cancel()
 	kv, err := p.kv(ctx)
 	if err != nil {
-		p.logf("the state %s cannot be read, so no node is asked to compose: %v", delivery.PlacementBucket, err)
-		return nil
+		return nil, fmt.Errorf("the state %s cannot be read: %w", delivery.PlacementBucket, err)
 	}
 	lister, err := kv.ListKeys(ctx)
+	if errors.Is(err, jetstream.ErrNoKeysFound) {
+		return nil, nil // a placement that places nothing
+	}
 	if err != nil {
-		p.logf("the state %s cannot be listed, so no node is asked to compose: %v", delivery.PlacementBucket, err)
-		return nil
+		return nil, fmt.Errorf("the state %s cannot be listed: %w", delivery.PlacementBucket, err)
 	}
 	seen := map[string]bool{}
 	for key := range lister.Keys() {
@@ -81,7 +83,7 @@ func (p placedNodes) Placed() []string {
 		out = append(out, n)
 	}
 	sort.Strings(out)
-	return out
+	return out, nil
 }
 
 // seatSandbox runs a compose job on the Go build seat's check stream, as the checker's own ask, and answers the

modules/mesh-delivery-checker/cmd/mesh-delivery-checker/main.go

@@ -176,9 +176,8 @@ func run(ctx context.Context, getenv func(string) string, logOut io.Writer) erro
 	}
 	verdicts := &manifestVerdicts{}
 	compositions := check.EngineCompositions{
-		Asker: engineAsks{call: caller.CallOn},
-		Placed: placedNodes{kv: func(at context.Context) (jetstream.KeyValue, error) { return js.KeyValue(at, delivery.PlacementBucket) },
-			logf: logger.Printf}.Placed,
+		Asker:    engineAsks{call: caller.CallOn},
+		Placed:   placedNodes{kv: func(at context.Context) (jetstream.KeyValue, error) { return js.KeyValue(at, delivery.PlacementBucket) }}.Placed,
 		Bound:    answerBound,
 		Sandbox:  seatSandbox{asker: buildrun.Asker{Publisher: asks, Waiter: waiter, Bound: stepBound}},
 		Verdicts: verdicts.all,

modules/mesh-delivery-checker/internal/check/engines.go

@@ -39,8 +39,9 @@ type Sandbox interface {
 // EngineCompositions is the checker's composition of a check (Examiner.Compositions).
 type EngineCompositions struct {
 	Asker EngineAsker
-	// Placed names every node in placement now (the seat mesh-placement's state).
-	Placed func() []string
+	// Placed names every node in placement now (the seat mesh-placement's state), or why it cannot be read: then no node
+	// can be asked, and the composition is not made (an error), never passed with nobody asked.
+	Placed func() ([]string, error)
 	// Bound is the answer bound (the setting compose-answer-bound); zero is delivery.ComposeAnswerBound.
 	Bound time.Duration
 	// Composer is the commit of the node-engine the mesh runs, named as what composed (ADR 0548 §6's running commit).
@@ -71,7 +72,9 @@ func (e EngineCompositions) Compose(ctx context.Context, c *Case) (delivery.Comp
 	}
 	var nodes []string
 	if e.Placed != nil {
-		nodes = e.Placed()
+		if nodes, err = e.Placed(); err != nil {
+			return delivery.CompositionVerdict{}, fmt.Errorf("the placement cannot be read, so no node can be asked: %w", err)
+		}
 	}
 	bound := e.Bound
 	if bound <= 0 {

modules/mesh-delivery-checker/internal/check/engines_test.go

@@ -81,7 +81,7 @@ func TestTheCompositionIsMadeFromTheEnginesAnswers(t *testing.T) {
 	key, _ := delivery.CompositionKey(cs.Group, cs.HeadsHash)
 	compose := func(e *fakeEngines, nodes ...string) delivery.CompositionVerdict {
 		t.Helper()
-		ec := EngineCompositions{Asker: e, Placed: func() []string { return nodes }, Bound: 50 * time.Millisecond,
+		ec := EngineCompositions{Asker: e, Placed: func() ([]string, error) { return nodes, nil }, Bound: 50 * time.Millisecond,
 			Composer: func() string { return strings.Repeat("7", 40) }}
 		v, err := ec.Compose(context.Background(), cs)
 		if err != nil {
@@ -149,7 +149,7 @@ func TestTheCompositionIsMadeFromTheEnginesAnswers(t *testing.T) {
 			return composer.Verdict{V: composer.Version, Cases: []composer.CaseVerdict{{Name: "change",
 				Machines: map[string]composer.Machine{node: {Composes: false, Reason: "the new composer refuses it"}}}}}
 		}}
-		ec := EngineCompositions{Asker: e, Placed: func() []string { return []string{"anchor"} }, Bound: time.Second,
+		ec := EngineCompositions{Asker: e, Placed: func() ([]string, error) { return []string{"anchor"}, nil }, Bound: time.Second,
 			Sandbox: sb, Manifests: digests{"web": "sha256:" + strings.Repeat("c", 64)}}
 		v, err := ec.Compose(context.Background(), host)
 		if err != nil {
@@ -178,4 +178,17 @@ func TestTheCompositionIsMadeFromTheEnginesAnswers(t *testing.T) {
 		}
 	})
 
+	// Review finding 4: a placement that cannot be read is no mesh with no node: the composition is not made, said.
+	t.Run("an unreadable placement makes no composition", func(t *testing.T) {
+		ec := EngineCompositions{Asker: &fakeEngines{}, Placed: func() ([]string, error) { return nil, errors.New("no such bucket") }}
+		if _, err := ec.Compose(context.Background(), cs); err == nil || !strings.Contains(err.Error(), "no such bucket") {
+			t.Errorf("an unreadable placement: %v", err)
+		}
+		b := newBench(t)
+		b.ex.Compositions = ec
+		v := b.judge(t, cs).Verdict
+		if v.Composition == nil || v.Composition.State != delivery.CompositionNotRun || !strings.Contains(v.Composition.Words, "no such bucket") {
+			t.Errorf("the verdict's composition = %+v", v.Composition)
+		}
+	})
 }

modules/mesh-delivery-checker/internal/check/judge.go

@@ -279,7 +279,9 @@ func (st *state) composed(ctx context.Context, v *delivery.Verdict) {
 	}
 	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).
 		st.j.logf("%s: the composition could not be made, so it is said as not run: %v", st.c.Key, err)
+		v.Composition = &delivery.Composition{State: delivery.CompositionNotRun, Words: "not made: " + err.Error()}
 		return
 	}
 	withComposition(v, comp)