Merge pull request '57/K2: The list of predefined keys, and a merge check refusing a key removed while a module names it (issue 661)' (#291) from pkg/57-k2-predefined-keys into feat/57-config-and-provisioning

Part 2 of 2: the check merge-check.sh runs, and its tests with a planted removal (hq ADR 0348 clause 7, issue 661).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

RELEASE-57.md

@@ -0,0 +1,6 @@
+# Release 57: config and provisioning
+
+This branch carries the work of hq design 57 (ADR 0348, ADR 0349) in this repository. Each package
+merges here through its own reviewed pull request; the branch reaches main as one pull request, in one
+delivery group with the branch of the same name in mesh-controller, mesh-host, mesh-sdk, mesh-catalog
+and hq, after a rehearsal in the lab. This file is deleted by the last commit before that pull request.

checks/predefined/go.mod

@@ -0,0 +1,3 @@
+module git.novox.be/novox/mesh-catalog/checks/predefined
+
+go 1.22

checks/predefined/main.go

@@ -0,0 +1,153 @@
+// Command predefined refuses a change to predefined.json that removes a key a module still names (novox/hq ADR
+// 0348 clause 7, design 57 package K2, issue 661).
+//
+// Mesh config and node config hold values only for the keys on the catalogue's list of predefined keys, at the
+// root of the repository; a module's config value may fall back to one, `"predefined": "<key>"`. A key removed
+// while a manifest still names it would leave that module falling back to nothing: the controller refuses its
+// build then, at the next merge that touches it. So the removal is refused here, in the pull request that makes
+// it, naming each module that names the key.
+//
+// **What it cannot know.** Whether a key still holds a value at the mesh or a node is mesh-config's to say (design
+// 57 package K3): it holds the values, and the catalogue's check reads no store. Until mesh-config refuses that
+// itself, removing a key nobody names is let through here, and said.
+//
+// The list it compares with is the trunk's at the merge base, given with -base; without one, nothing was removed.
+// The controller the mesh runs reads the list itself (type, validation, every manifest against it) in the
+// module check; this check reads only its keys.
+//
+//	go run . [-base <the trunk's predefined.json>] <catalogue root>
+//
+// Exits 1 on a finding.
+package main
+
+import (
+	"encoding/json"
+	"errors"
+	"flag"
+	"fmt"
+	"io/fs"
+	"os"
+	"path/filepath"
+	"sort"
+	"strings"
+)
+
+// File is the list's file at the root of the catalogue.
+const File = "predefined.json"
+
+// Keys is the keys of a list, read as a JSON object; an absent file has none.
+func Keys(path string) (map[string]bool, error) {
+	raw, err := os.ReadFile(path)
+	if errors.Is(err, fs.ErrNotExist) {
+		return map[string]bool{}, nil
+	}
+	if err != nil {
+		return nil, err
+	}
+	var keys map[string]json.RawMessage
+	if err := json.Unmarshal(raw, &keys); err != nil {
+		return nil, fmt.Errorf("%s is not a JSON object of keys: %w", path, err)
+	}
+	out := map[string]bool{}
+	for k := range keys {
+		out[k] = true
+	}
+	return out, nil
+}
+
+// Named is, per predefined key, the modules whose manifests name it: under `config`, or its alias `settings`.
+func Named(root string) (map[string][]string, error) {
+	manifests, err := filepath.Glob(filepath.Join(root, "modules", "*", "module.json"))
+	if err != nil {
+		return nil, err
+	}
+	sort.Strings(manifests)
+	named := map[string][]string{}
+	for _, path := range manifests {
+		raw, err := os.ReadFile(path)
+		if err != nil {
+			return nil, err
+		}
+		var m struct {
+			Module   string                     `json:"module"`
+			Config   map[string]json.RawMessage `json:"config"`
+			Settings map[string]json.RawMessage `json:"settings"`
+		}
+		if err := json.Unmarshal(raw, &m); err != nil {
+			return nil, fmt.Errorf("%s: %w", path, err)
+		}
+		for _, declared := range []map[string]json.RawMessage{m.Config, m.Settings} {
+			for _, d := range declared {
+				var value struct {
+					Predefined string `json:"predefined"`
+				}
+				if json.Unmarshal(d, &value) == nil && value.Predefined != "" {
+					named[value.Predefined] = append(named[value.Predefined], m.Module)
+				}
+			}
+		}
+	}
+	return named, nil
+}
+
+// Removed is each key the base list holds and the tree's does not, with the modules still naming it; a removed
+// key nobody names is answered with none.
+func Removed(base, tree map[string]bool, named map[string][]string) map[string][]string {
+	out := map[string][]string{}
+	for k := range base {
+		if !tree[k] {
+			out[k] = named[k]
+		}
+	}
+	return out
+}
+
+func main() {
+	basePath := flag.String("base", "", "the trunk's predefined.json at the merge base")
+	flag.Parse()
+	if flag.NArg() != 1 {
+		fmt.Fprintln(os.Stderr, "usage: predefined [-base <the trunk's predefined.json>] <catalogue root>")
+		os.Exit(2)
+	}
+	root := flag.Arg(0)
+	tree, err := Keys(filepath.Join(root, File))
+	if err != nil {
+		fmt.Println(err)
+		os.Exit(1)
+	}
+	if *basePath == "" {
+		fmt.Printf("%s: %d key(s); no trunk list given, so none was removed\n", File, len(tree))
+		return
+	}
+	base, err := Keys(*basePath)
+	if err != nil {
+		fmt.Println(err)
+		os.Exit(1)
+	}
+	named, err := Named(root)
+	if err != nil {
+		fmt.Println(err)
+		os.Exit(1)
+	}
+	removed := Removed(base, tree, named)
+	keys := make([]string, 0, len(removed))
+	for k := range removed {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	failed := false
+	for _, k := range keys {
+		if modules := removed[k]; len(modules) > 0 {
+			failed = true
+			fmt.Printf("%s: the key %q is removed and still named by %s: move each off it first (novox/hq ADR 0348 "+
+				"clause 7)\n", File, k, strings.Join(modules, ", "))
+		} else {
+			fmt.Printf("%s: the key %q is removed and no module names it; whether the mesh or a node still holds a "+
+				"value for it is mesh-config's to refuse, not this check's\n", File, k)
+		}
+	}
+	if failed {
+		os.Exit(1)
+	}
+	fmt.Printf("%s: %d key(s), none removed that a module names\n", File, len(tree))
+}

checks/predefined/main_test.go

@@ -0,0 +1,100 @@
+package main
+
+import (
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// A catalogue whose dunst falls back to font-size, and the trunk's list holding font-size and
+// agent-managed-settings.
+func aCatalogue(t *testing.T, list string) (root, base string) {
+	t.Helper()
+	root = t.TempDir()
+	write := func(path, text string) {
+		t.Helper()
+		if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(path, []byte(text), 0o644); err != nil {
+			t.Fatal(err)
+		}
+	}
+	write(filepath.Join(root, "modules", "dunst", "module.json"), `{"module": "dunst", "config": {"font-size":
+	  {"type": "number", "description": "points", "default": 10, "predefined": "font-size"}}}`)
+	write(filepath.Join(root, "modules", "claude-code", "module.json"), `{"module": "claude-code", "settings": {
+	  "managed_settings": {"kind": "preference", "default": "{}", "why": "none"}}}`)
+	write(filepath.Join(root, "modules", "plain", "module.json"), `{"module": "plain"}`)
+	if list != "" {
+		write(filepath.Join(root, File), list)
+	}
+	base = filepath.Join(t.TempDir(), "base.json")
+	write(base, `{"font-size": {"type": "number"}, "agent-managed-settings": {"type": "object"}}`)
+	return root, base
+}
+
+func run(t *testing.T, args ...string) (string, error) {
+	t.Helper()
+	out, err := exec.Command("go", append([]string{"run", "."}, args...)...).CombinedOutput()
+	return string(out), err
+}
+
+func TestARemovalOfAKeyAModuleStillNamesIsRefused(t *testing.T) {
+	// The planted removal: font-size goes while dunst still falls back to it.
+	root, base := aCatalogue(t, `{"agent-managed-settings": {"type": "object"}}`)
+	out, err := run(t, "-base", base, root)
+	if err == nil || !strings.Contains(out, `the key "font-size" is removed and still named by dunst`) {
+		t.Fatalf("a removal of a named key passed:\n%s", out)
+	}
+}
+
+func TestARemovalNobodyNamesPassesAndSaysWhatItCannotKnow(t *testing.T) {
+	root, base := aCatalogue(t, `{"font-size": {"type": "number"}}`)
+	out, err := run(t, "-base", base, root)
+	if err != nil || !strings.Contains(out, `"agent-managed-settings" is removed and no module names it`) ||
+		!strings.Contains(out, "mesh-config's to refuse") {
+		t.Fatalf("a removal nobody names: %v\n%s", err, out)
+	}
+}
+
+func TestAListUnchangedOrGrownPasses(t *testing.T) {
+	for _, list := range []string{
+		`{"font-size": {"type": "number"}, "agent-managed-settings": {"type": "object"}}`,
+		`{"font-size": {"type": "number"}, "agent-managed-settings": {"type": "object"}, "accent": {"type": "string"}}`,
+	} {
+		root, base := aCatalogue(t, list)
+		if out, err := run(t, "-base", base, root); err != nil || !strings.Contains(out, "none removed") {
+			t.Fatalf("%s: %v\n%s", list, err, out)
+		}
+	}
+	// No trunk list given: nothing was removed.
+	root, _ := aCatalogue(t, `{}`)
+	if out, err := run(t, root); err != nil || !strings.Contains(out, "none was removed") {
+		t.Fatalf("without a base: %v\n%s", err, out)
+	}
+}
+
+func TestTheFileRemovedWholeIsARemovalOfEveryKey(t *testing.T) {
+	root, base := aCatalogue(t, "")
+	out, err := run(t, "-base", base, root)
+	if err == nil || !strings.Contains(out, `"font-size" is removed and still named by dunst`) {
+		t.Fatalf("the list removed whole passed:\n%s", out)
+	}
+}
+
+func TestNamedReadsBothForms(t *testing.T) {
+	root, _ := aCatalogue(t, "")
+	if err := os.WriteFile(filepath.Join(root, "modules", "plain", "module.json"), []byte(`{"module": "plain",
+	  "settings": {"size": {"predefined": "font-size"}}}`), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	named, err := Named(root)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := strings.Join(named["font-size"], ","); got != "dunst,plain" {
+		t.Fatalf("font-size named by %s", got)
+	}
+}

merge-check.sh

@@ -33,6 +33,10 @@
 #   6. the secrets (novox/hq issue 342): no mergeable file fills a key with a vault secret that a setting could
 #      replace — the key is `protected`, or left out of the file's `takes-settings`. checks/secrets/known names
 #      the nested ones the controller cannot protect alone yet, and only shrinks.
+#   7. the list of predefined keys (novox/hq ADR 0348 clause 7): a change to predefined.json removes no key a
+#      module still names, judged against the trunk's copy at the merge base. Whether a removed key still holds a
+#      value at the mesh or a node is mesh-config's to refuse; this check reads no store. The list itself and
+#      every manifest against it are judged by the controller's module check in step 1.
 set -eu
 
 (cd checks/words && go test -count=1 . && go run . ../..)
@@ -51,6 +55,18 @@ else
 	echo "NOT COMPARED WITH THE TRUNK: no merge base with $secrets_ref; checks/secrets/known is held to the list it was first written with"
 fi
 (cd checks/secrets && go test -count=1 . && if [ -n "$secrets_base" ]; then go run . -base "$secrets_base" ../..; else go run . ../..; fi)
+# The list of predefined keys (novox/hq ADR 0348 clause 7): no key removed that a module still names. The trunk's
+# list at the merge base is what it is judged against; a trunk without the file had none.
+predefined_base=""
+if merge_base=$(git merge-base HEAD "origin/${MESH_CHECK_BASE:-main}" 2>/dev/null); then
+	predefined_base=$(mktemp)
+	if ! git show "$merge_base:predefined.json" >"$predefined_base" 2>/dev/null; then
+		echo '{}' >"$predefined_base"
+	fi
+else
+	echo "NOT COMPARED WITH THE TRUNK: no merge base with origin/${MESH_CHECK_BASE:-main}; no removal from predefined.json can be seen"
+fi
+(cd checks/predefined && go test -count=1 . && if [ -n "$predefined_base" ]; then go run . -base "$predefined_base" ../..; else go run . ../..; fi)
 # The lists that narrow the paths check may not grow unseen: the base branch's copies are what they are
 # judged against (MESH_CHECK_BASE, main when unset), read from the clone the check runs in.
 base_lists=""

modules/influxdb/module.json

@@ -26,12 +26,24 @@
       "why": "queries, writes and the web UI, over http; consumers granted influxdb-api sign in with the mesh's credential, and a name is a route grant"
     }
   ],
+  "settings": {
+    "org": {
+      "kind": "preference",
+      "default": "mesh",
+      "why": "the organisation influxdb-api names to its consumers; mesh is the one influxdb makes at its first start (server.env), and a machine whose server was set up with another sets its own (novox/hq issue 696, ADR 0348 clause 11)"
+    },
+    "bucket": {
+      "kind": "preference",
+      "default": "default",
+      "why": "the bucket influxdb-api names to its consumers; default is the one influxdb makes at its first start (server.env), and a machine whose server was set up with another sets its own (novox/hq issue 696, ADR 0348 clause 11)"
+    }
+  },
   "serves": {
     "influxdb-api": {
       "scheme": "http",
       "port": 8086,
-      "org": "mesh",
-      "bucket": "default"
+      "org": "${setting:org}",
+      "bucket": "${setting:bucket}"
     }
   },
   "receives": {
@@ -124,7 +136,13 @@
       "path": "${dir:mesh-state}/config.json",
       "mode": "0600",
       "content": "{}\n",
-      "merge": "json"
+      "merge": "json",
+      "takes-settings": [
+        "bucket",
+        "org",
+        "url"
+      ],
+      "trusted": true
     }
   ],
   "requires": [

modules/mailu/module.json

@@ -371,7 +371,7 @@
       "id": "resolver",
       "type": "container",
       "name": "mailu-resolver",
-      "image": "ghcr.io/mailu/unbound@sha256:2c037bc5c988b5c0ab7e1b6e6ffa762e14ba1e45c96281ac87f8b9be8eee8b6e",
+      "image": "ghcr.io/mailu/unbound@sha256:3a0fdfb364a63f4f9259526e013c1ef40f5f14de3621ce1560804b3a5909584a",
       "health": {
         "kind": "runtime"
       },

predefined.json

@@ -0,0 +1,13 @@
+{
+  "agent-managed-settings": {
+    "type": "object",
+    "description": "The managed settings every agent session on a node is held to (Claude Code's managed-settings.json), as a JSON object.",
+    "trusted": true
+  },
+  "font-size": {
+    "type": "number",
+    "description": "The size a desktop program shows its text in, in points.",
+    "minimum": 6,
+    "maximum": 72
+  }
+}