Refuse a manifest judge in mesh-sdk's own check, and never end the data items' read on one module (ADR 0578 §3, §4)

Review round 1 (issue 1585): L1, DataItemsHeld holds a module by the culprit rule PlansHeld uses, a held module's
data left out, so an item its composition refuses holds that module alone; L2, mesh-sdk's own tests scan its go/
tree as mesh-catalog's checks/sdkjudge does, with the same list of composition's refusals. Two tests merged into one.

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

go/catalogue/setting_defaults_test.go

@@ -31,6 +31,7 @@ func parsed(t *testing.T, m Manifest) error {
 }
 
 func TestAnUnsetPreferenceTakesItsDefault(t *testing.T) {
+	t.Run("the node over the mesh over the default", theNodeOverTheMeshOverTheDefault)
 	m := notifier()
 	for _, layers := range [][]Layer{nil, {{From: MeshWideLayer, Values: map[string]any{}}}} {
 		file := map[string]any{}
@@ -49,7 +50,7 @@ func TestAnUnsetPreferenceTakesItsDefault(t *testing.T) {
 	}
 }
 
-func TestTheNodeOverTheMeshOverTheDefault(t *testing.T) {
+func theNodeOverTheMeshOverTheDefault(t *testing.T) {
 	m := notifier()
 	layers := []Layer{
 		{From: MeshWideLayer, Values: map[string]any{"width": float64(300)}},

go/compose/frombus/frombus.go

@@ -545,10 +545,16 @@ func PlansHeld(ctx context.Context, r Reader, mesh Mesh, s Scope) (plans map[str
 // plansHeldOf is the plans of node over in, every module in held held, and each module a refusal is of held too until
 // the plans compose (culprit's rule over the Planning mode: its stub first, then its bare stub).
 func plansHeldOf(in compose.Input, node string, held map[string]string) (map[string]catalogue.Plan, error) {
+	return heldOf(in, node, held, func(in compose.Input) (map[string]catalogue.Plan, error) { return plansIn(in, node, compose.Planning) })
+}
+
+// heldOf composes compose1 over in with every module in held held, and each module a refusal is of held too until it
+// composes (culprit's rule: its stub first, then its bare stub); what compose1 answers of a held module is left out.
+// Plans and data items are composed so (PlansHeld, DataItemsHeld): one module never ends the read (novox/hq ADR 0578 §3).
+func heldOf[V any](in compose.Input, node string, held map[string]string, compose1 func(compose.Input) (map[string]V, error)) (map[string]V, error) {
 	for _, m := range sortedKeys(held) {
 		in = stubbed(in, node, m, "")
 	}
-	compose1 := func(in compose.Input) (map[string]catalogue.Plan, error) { return plansIn(in, node, compose.Planning) }
 	// Each try holds one module more or leaves out all of one held module's offer: bounded by twice the modules placed.
 	for tries := 0; tries <= 2*len(assignedOn(in, node))+1; tries++ {
 		plans, err := compose1(in)
@@ -585,7 +591,7 @@ func plansHeldOf(in compose.Input, node string, held map[string]string) (map[str
 		}
 		in = stubbed(in, node, module, leave)
 	}
-	return nil, fmt.Errorf("frombus: the plans of %s do not compose with %d module(s) held: %s", node, len(held),
+	return nil, fmt.Errorf("frombus: what %s composes does not compose with %d module(s) held: %s", node, len(held),
 		strings.Join(sortedKeys(held), ", "))
 }
 
@@ -1057,9 +1063,34 @@ func DataItemsHeld(ctx context.Context, r Reader, mesh Mesh, s Scope) (items map
 	if err != nil {
 		return nil, nil, held, err
 	}
-	items, unplaced, err = compose.DataItems(in, s.Node)
-	for m := range held {
-		delete(items, m)
-	}
+	// Held module by module, as PlansHeld holds them: a refused stub or one module's composition refusal (an item's
+	// template) holds that module and never ends the read.
+	items, err = heldOf(in, s.Node, held, func(in compose.Input) (map[string][]catalogue.PlacedDataItem, error) {
+		var err error
+		items, unplaced, err = compose.DataItems(withoutHeldData(in, s.Node), s.Node)
+		return items, err
+	})
 	return items, unplaced, held, err
 }
+
+// withoutHeldData is in with no data item of a module held on node: none of a held module's items is measured, so its
+// data never refuses the rest (its stub keeps its data for the backups the plans compose).
+func withoutHeldData(in compose.Input, node string) compose.Input {
+	out := in
+	out.Nodes = append([]compose.Node(nil), in.Nodes...)
+	for i := range out.Nodes {
+		n := &out.Nodes[i]
+		if n.Name != node || len(n.Held) == 0 {
+			continue
+		}
+		kept := maps.Clone(n.Kept)
+		for m := range n.Held {
+			if man, ok := kept[m]; ok {
+				man.Data = nil
+				kept[m] = man
+			}
+		}
+		n.Kept = kept
+	}
+	return out
+}

go/compose/frombus/frombustest/frombustest_test.go

@@ -29,16 +29,25 @@ func TestTheProvisionedMeshHandsAPlanWithItsProvision(t *testing.T) {
 		// refused is the indexer's verdict on bad, a problem its judge found (novox/hq ADR 0578 §1): Plans then holds
 		// backup too, and never ends the read.
 		refused string
+		// dataHeld says the data items' read holds backup too: a refusal it meets (a refused manifest, an unusable build,
+		// an item composition refuses), not one only a plan meets.
+		dataHeld bool
+		// dataOnly checks the data items' read alone: the other reads blame restic, whose templates collect the item.
+		dataOnly bool
 	}{
 		{name: "every module composes"},
 		{name: "a built manifest that cannot be used",
 			bad:   `{"module":"backup","version":"1","build":{"artifacts":[{"name":"files","kind":"archive","from":"files"}]}}`,
-			built: `{"module":"backup","version":"1","requires":"amqp"}`, why: "backup"},
+			built: `{"module":"backup","version":"1","requires":"amqp"}`, why: "backup", dataHeld: true},
 		{name: "a plan that refuses",
 			bad: `{"module":"backup","version":"1","resources":[{"id":"conf","type":"file","path":"/etc/backup.conf",` +
 				`"mode":"0600","content":"key=${secret:nope}"}]}`, why: "${secret:nope}"},
 		{name: "an indexer's manifest the mesh refuses", bad: unusableItem, why: `"shows" is none of them`,
-			refused: `"shows" is none of them`},
+			refused: `"shows" is none of them`, dataHeld: true},
+		// ADR 0578 §3: an accepted manifest whose data item its composition refuses is held by the culprit rule, in every read.
+		{name: "a data item its composition refuses",
+			bad: `{"module":"backup","version":"1","data":{"own":[{"id":"d","path":"${dir:nope}","class":"cache"}]}}`,
+			why: "nope", dataHeld: true, dataOnly: true},
 		{name: "an indexer's manifest the mesh refuses, placed elsewhere", bad: unusableItem, elsewhere: true,
 			refused: `"shows" is none of them`},
 	} {
@@ -65,6 +74,13 @@ func TestTheProvisionedMeshHandsAPlanWithItsProvision(t *testing.T) {
 				b.Builds["backup "+fp] = &delivery.BuilderBuild{State: delivery.BuildDone, Manifest: builtDigest,
 					Artifacts: []delivery.BuiltArtifact{{Name: "files", Kind: "archive", Reference: "artifact-store://backup/files/blobs/x", Digest: "sha256:x"}}}
 			}
+			if row.dataOnly {
+				items, _, iheld, err := frombus.DataItemsHeld(context.Background(), b, frombus.Mesh{}, frombus.Scope{Node: Node})
+				if why, ok := iheld["backup"]; err != nil || !ok || !strings.Contains(why, row.why) || len(items["restic"]) == 0 {
+					t.Errorf("%s: data items %v held %v: %v", row.name, items, iheld, err)
+				}
+				continue
+			}
 			plans, err := frombus.Plans(context.Background(), b, frombus.Mesh{}, frombus.Scope{Node: Node})
 			switch {
 			case row.refused != "" && (err != nil || !planned(plans, "restic") || planned(plans, "backup")):
@@ -111,6 +127,13 @@ func TestTheProvisionedMeshHandsAPlanWithItsProvision(t *testing.T) {
 		if _, planned := plans["backup"]; planned {
 			t.Errorf("%s: the held module has a plan", row.name)
 		}
+		// The data items are read by the same rule (ADR 0578 §3: disk-load's read): the bad module held, the read whole.
+		items, _, iheld, err := frombus.DataItemsHeld(context.Background(), b, frombus.Mesh{}, frombus.Scope{Node: Node})
+		// A refusal only a plan meets (a file's secret) holds nothing here: data items render no such file.
+		mustHold := row.dataHeld
+		if why, ok := iheld["backup"]; err != nil || ok != mustHold || mustHold && !strings.Contains(why, row.why) || items["backup"] != nil {
+			t.Errorf("%s: data items %v held %v: %v", row.name, items, iheld, err)
+		}
 	}
 }
 

go/judge_free_test.go

@@ -0,0 +1,175 @@
+// The same scan as mesh-catalog's checks/sdkjudge, run by mesh-sdk's own check (novox/hq ADR 0578 §4: the change that
+// adds a manifest judge to mesh-sdk is refused), so a change to mesh-sdk alone is refused too. Copied from mesh-catalog
+// feat/judge-once checks/sdkjudge/main.go; the two lists of composition's refusals are kept the same.
+package stdio
+
+import (
+	"fmt"
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"io/fs"
+	"path/filepath"
+	"regexp"
+	"sort"
+	"strings"
+	"testing"
+)
+
+// composition is every function or table the check lets stay in mesh-sdk, as "path-from-go/:name", and why. It names no
+// manifest judgement: each is the composer's refusal of composition itself (ADR 0578 clause 5).
+var composition = map[string]string{
+	"catalogue/consumers.go:consumersPlaceholderProblems":     "composition: rendering a consumer's template",
+	"catalogue/collections.go:collectedPlaceholderProblems":   "composition: placeholders a contribution renders",
+	"catalogue/environment_into.go:placeholderProblems":       "composition: placeholders a contribution renders",
+	"catalogue/seat_contributions.go:seatPlaceholderProblems": "composition: placeholders a contribution renders",
+	"catalogue/collections.go:itemTemplateProblem":            "composition: filling an item's template",
+	"catalogue/setting_takes.go:takenSettingsProblems":        "composition: whether stored settings still compose",
+	"catalogue/template.go:HeldTemplateProblem":               "composition: the take-in refusal of a resolved build",
+}
+
+// judgeProblems is what mesh-sdk's Go tree under root (its go/ directory) holds of a manifest judge, one line each, but for
+// what composition lets stay (the map the caller gives; the test of the real tree gives the package's), and the
+// entries of composition it no longer holds.
+func judgeProblems(root string, composition map[string]string) ([]string, error) {
+	var out []string
+	seen := map[string]bool{}
+	err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		if d.IsDir() {
+			if n := d.Name(); n == "testdata" || n == "vendor" || strings.HasPrefix(n, ".") {
+				return filepath.SkipDir
+			}
+			return nil
+		}
+		if !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") {
+			return nil
+		}
+		rel, _ := filepath.Rel(root, path)
+		file, err := parser.ParseFile(token.NewFileSet(), path, nil, parser.SkipObjectResolution)
+		if err != nil {
+			return fmt.Errorf("%s: %w", rel, err)
+		}
+		inCatalogue := filepath.Base(filepath.Dir(path)) == "catalogue"
+		for _, d := range file.Decls {
+			switch d := d.(type) {
+			case *ast.FuncDecl:
+				if why := judgeFunc(d); why != "" {
+					out = append(out, note(composition, seen, rel, d.Name.Name, why))
+				}
+			case *ast.GenDecl:
+				if !inCatalogue || d.Tok != token.VAR {
+					continue
+				}
+				for _, spec := range d.Specs {
+					v, _ := spec.(*ast.ValueSpec)
+					if v == nil {
+						continue
+					}
+					for i, name := range v.Names {
+						if i < len(v.Values) && allowName.MatchString(name.Name) && isTable(v.Values[i]) {
+							out = append(out, note(composition, seen, rel, name.Name,
+								"a table of what a manifest may declare (an allow-list)"))
+						}
+					}
+				}
+			}
+		}
+		return nil
+	})
+	if err != nil {
+		return nil, err
+	}
+	var kept []string
+	for _, line := range out {
+		if line != "" {
+			kept = append(kept, line)
+		}
+	}
+	for key, why := range composition {
+		if !seen[key] {
+			kept = append(kept, fmt.Sprintf("composition lists %s (%s), which mesh-sdk no longer holds: take it off the list", key, why))
+		}
+	}
+	sort.Strings(kept)
+	return kept, nil
+}
+
+// note is the line refusing name in rel for why, or "" when composition lets it stay.
+func note(composition map[string]string, seen map[string]bool, rel, name, why string) string {
+	key := rel + ":" + name
+	if _, ok := composition[key]; ok {
+		seen[key] = true
+		return ""
+	}
+	return fmt.Sprintf("%s: %s is %s; mesh-sdk carries the contract only, the judge is the indexer's and the checker's (ADR 0578 §4)", rel, name, why)
+}
+
+// allowName is what names a table of what is allowed.
+var allowName = regexp.MustCompile(`(?i)allow|permit|removable|reserved|forbid|denied`)
+
+// isTable says whether an expression is a map or a list literal of more than one entry.
+func isTable(e ast.Expr) bool {
+	c, ok := e.(*ast.CompositeLit)
+	if !ok || len(c.Elts) < 2 {
+		return false
+	}
+	switch c.Type.(type) {
+	case *ast.MapType, *ast.ArrayType:
+		return true
+	}
+	return false
+}
+
+// judgeFunc says why a function is a manifest judge, or "".
+func judgeFunc(f *ast.FuncDecl) string {
+	name := f.Name.Name
+	if judgeEntry.MatchString(name) {
+		return "a function that judges a manifest"
+	}
+	if !strings.HasSuffix(name, "Problems") && !strings.HasSuffix(name, "Problem") {
+		return ""
+	}
+	if takes(f.Recv) || takes(f.Type.Params) {
+		return "a function that gathers a manifest's problems"
+	}
+	return ""
+}
+
+// judgeEntry is the names of the judge's entry points: the strict parsers and the validators of a manifest or a shelf.
+var judgeEntry = regexp.MustCompile(`^(ParseManifest(With|ForInputs)?|Validate(Manifest|Shelf)\w*)$`)
+
+// takes says whether a list of parameters names a Manifest or a Shelf anywhere in a type.
+func takes(fields *ast.FieldList) bool {
+	if fields == nil {
+		return false
+	}
+	found := false
+	for _, f := range fields.List {
+		ast.Inspect(f.Type, func(n ast.Node) bool {
+			if id, ok := n.(*ast.Ident); ok && (id.Name == "Manifest" || id.Name == "Shelf") {
+				found = true
+			}
+			return !found
+		})
+	}
+	return found
+}
+
+// mesh-sdk holds no manifest judge: no function gathering a manifest's problems, no judge entry point and no allow-list
+// table in the catalogue, but for composition's own refusals, each listed with why (ADR 0578 §4, §5).
+func TestMeshSDKHoldsNoManifestJudge(t *testing.T) {
+	root, err := filepath.Abs(".")
+	if err != nil {
+		t.Fatal(err)
+	}
+	problems, err := judgeProblems(root, composition)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, p := range problems {
+		t.Error(p)
+	}
+}