From 286257d80257ecf49ee9fe0fdf9f8518afb05378 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 17:47:05 +0200 Subject: [PATCH 1/3] Let a mergeable file name the setting keys it takes (hq issue 168, ADR 0326) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every key of a module's layers merged into every mergeable file it owns: the mesh's own words, a key meant for a ${setting:} in another file, a key for a second configuration file. A module could not own two configurable files. A file now names its keys in takes-settings and takes only those; a key no file names is stray where it is set; a module that declares a setting names the keys of every mergeable file, and one that declares none keeps today's form (ADR 0164 §15). --- internal/catalogue/manifest.go | 2 + internal/catalogue/setting_takes.go | 187 +++++++++++++++++++ internal/catalogue/setting_takes_test.go | 217 +++++++++++++++++++++++ internal/catalogue/settings.go | 35 +++- 4 files changed, 438 insertions(+), 3 deletions(-) create mode 100644 internal/catalogue/setting_takes.go create mode 100644 internal/catalogue/setting_takes_test.go diff --git a/internal/catalogue/manifest.go b/internal/catalogue/manifest.go index 024d850a..6e8b398f 100644 --- a/internal/catalogue/manifest.go +++ b/internal/catalogue/manifest.go @@ -1693,6 +1693,8 @@ func ParseManifest(raw []byte) (Manifest, error) { problems = append(problems, LogProblems(m)...) // And the defaults it gives its settings (setting_defaults.go, novox/hq ADR 0262). problems = append(problems, SettingProblems(m)...) + // And which setting keys each mergeable file takes (setting_takes.go, novox/hq ADR 0326). + problems = append(problems, TakenSettingsProblems(m)...) wellFormed := true for _, c := range m.Claims { if !name.MatchString(c.Name) { diff --git a/internal/catalogue/setting_takes.go b/internal/catalogue/setting_takes.go new file mode 100644 index 00000000..6215bd33 --- /dev/null +++ b/internal/catalogue/setting_takes.go @@ -0,0 +1,187 @@ +package catalogue + +import ( + "fmt" + "strings" + "unicode" +) + +// A mergeable file names the setting keys it takes (novox/hq ADR 0326, issue 168). +// +// Before this, every key of a module's layers merged into every one of its mergeable files: the +// mesh's own words, a key meant for a `${setting:}` in another file, a key for a second +// configuration file. So a module could not own two configurable files, and every receiver had to +// ignore what it did not know. ADR 0164 §11 decided that a setting names the files that read it; this +// is that relation, stated from the file's side, so the operator's own values stay declared nowhere +// (ADR 0262 §2). + +// TakesSettingsField is the key a mergeable file carries to name the top-level setting keys it takes. +const TakesSettingsField = "takes-settings" + +// takenKeys is the keys a resource names in takes-settings, and whether it names any list at all. An +// empty list is a list: that file takes no setting. +func takenKeys(resource map[string]any) (map[string]bool, bool) { + raw, said := resource[TakesSettingsField] + if !said { + return nil, false + } + keys := map[string]bool{} + for _, k := range stringsOf(raw) { + keys[k] = true + } + return keys, true +} + +// onlyTaken is the layers with every key the file does not name left out. The file's own defaults and +// what the mesh reads itself are not touched here: settle already keeps both out of a file. +func onlyTaken(layers []Layer, taken map[string]bool) []Layer { + kept := make([]Layer, 0, len(layers)) + for _, layer := range layers { + values := map[string]any{} + for key, value := range layer.Values { + if taken[key] { + values[key] = value + } + } + kept = append(kept, Layer{From: layer.From, Values: values, Default: layer.Default}) + } + return kept +} + +// namesItsKeys says whether a module's mergeable files name the keys they take (ADR 0326 §2): true +// only when it has one and every one carries takes-settings. A module with a silent mergeable file +// keeps today's form (§3), where every key reaches every mergeable file. +func namesItsKeys(m Manifest) bool { + merging := 0 + for _, r := range m.Resources { + if how, _ := r["merge"].(string); how == "" { + continue + } + merging++ + if _, said := r[TakesSettingsField]; !said { + return false + } + } + return merging > 0 +} + +// TakenSettingsProblems is every way a definition's takes-settings is wrong (ADR 0326 §2 and §4), in +// its own words: on a file that does not merge, not a list of keys, a key named twice, one the mesh +// reads itself, one the file keeps for itself, or one mergeable file naming its keys beside another +// that names none. +func TakenSettingsProblems(m Manifest) []string { + problems := takenSettingsProblems(m) + // **A module that declares a setting names the keys of every mergeable file** (ADR 0326 §3, ADR + // 0164 §11 and §15): only a module that declares none keeps today's form. Said where a definition + // is checked and registered, never where a stored one is composed: a definition stored before + // this record is brought into the form by its next change, not left out of a machine meanwhile. + if len(m.Settings) > 0 { + for _, r := range m.Resources { + how, _ := r["merge"].(string) + if _, said := r[TakesSettingsField]; how != "" && !said { + problems = append(problems, fmt.Sprintf("%s declares settings, and its mergeable %v names no %s: "+ + "a module that declares a setting names the keys each mergeable file takes, an empty list "+ + "for none (novox/hq ADR 0326, ADR 0164 §11)", m.Module, r["id"], TakesSettingsField)) + } + } + } + return problems +} + +// takenSettingsProblems is what TakenSettingsProblems says that a stored definition is judged by too. +func takenSettingsProblems(m Manifest) []string { + var problems []string + var naming, silent []string + for _, r := range m.Resources { + id := fmt.Sprint(r["id"]) + how, _ := r["merge"].(string) + raw, said := r[TakesSettingsField] + if how == "" { + if said { + problems = append(problems, fmt.Sprintf("%s's %s says %s and does not merge: only a mergeable "+ + "file takes settings by key (novox/hq ADR 0326)", m.Module, id, TakesSettingsField)) + } + continue + } + if !said { + silent = append(silent, id) + continue + } + naming = append(naming, id) + list, isList := raw.([]any) + if !isList { + problems = append(problems, fmt.Sprintf("%s's %s: %s is a list of setting keys, and this is %T", + m.Module, id, TakesSettingsField, raw)) + continue + } + protected := map[string]bool{} + for _, k := range stringsOf(r["protected"]) { + protected[k] = true + } + seen := map[string]bool{} + for _, item := range list { + key, isString := item.(string) + switch { + case !isString || !usableKey(key): + problems = append(problems, fmt.Sprintf("%s's %s: %v in %s is not a usable setting key", + m.Module, id, item, TakesSettingsField)) + case seen[key]: + problems = append(problems, fmt.Sprintf("%s's %s names %q twice in %s", + m.Module, id, key, TakesSettingsField)) + case meshWords[key]: + problems = append(problems, fmt.Sprintf("%s's %s names %q in %s, and the mesh reads %q itself "+ + "(novox/hq ADR 0164 §14)", m.Module, id, key, TakesSettingsField, key)) + case protected[key]: + problems = append(problems, fmt.Sprintf("%s's %s names %q in %s and keeps it as protected: a "+ + "key is the file's or the setting's, never both", m.Module, id, key, TakesSettingsField)) + } + if isString { + seen[key] = true + } + } + } + if len(naming) > 0 { + for _, id := range silent { + problems = append(problems, fmt.Sprintf("%s's %s merges and names no %s while %s does: once one "+ + "mergeable file of a module names the keys it takes, every one does, an empty list for none "+ + "(novox/hq ADR 0326)", m.Module, id, TakesSettingsField, strings.Join(naming, ", "))) + } + } + return problems +} + +// usableKey is a key a file can be said to take: not empty, and no space, line end or control +// character. Not the lower-case rule of a `${setting:}`: a file's own keys are its software's +// spelling, such as `timeZone`. +func usableKey(key string) bool { + if key == "" { + return false + } + for _, r := range key { + if unicode.IsSpace(r) || unicode.IsControl(r) || !unicode.IsPrint(r) { + return false + } + } + return true +} + +// takenSaid is what a stray key's message adds for a module whose files name their keys: which keys +// each file takes, so the person sees where the key would have had to be named. +func takenSaid(m Manifest) string { + if !namesItsKeys(m) { + return "" + } + var files []string + for _, r := range m.Resources { + if how, _ := r["merge"].(string); how == "" { + continue + } + keys := stringsOf(r[TakesSettingsField]) + if len(keys) == 0 { + files = append(files, fmt.Sprintf("%v takes none", r["id"])) + continue + } + files = append(files, fmt.Sprintf("%v takes %s", r["id"], strings.Join(keys, ", "))) + } + return " (" + strings.Join(files, "; ") + ")" +} diff --git a/internal/catalogue/setting_takes_test.go b/internal/catalogue/setting_takes_test.go new file mode 100644 index 00000000..fc59f79b --- /dev/null +++ b/internal/catalogue/setting_takes_test.go @@ -0,0 +1,217 @@ +package catalogue + +import ( + "encoding/json" + "strings" + "testing" +) + +// twoFiles is a module that owns two configurable files, which issue 168 said no module could: +// every key of every layer reached both. +func twoFiles(app, sidecar []any) Manifest { + appFile := map[string]any{"id": "app", "type": "file", "path": "/etc/app.json", "merge": MergeJSON, + "content": `{"server":{"port":8080},"search":{"safe":1}}`} + sideFile := map[string]any{"id": "sidecar", "type": "file", "path": "/etc/sidecar.json", "merge": MergeJSON, + "content": `{"url":"http://127.0.0.1:8080"}`} + if app != nil { + appFile[TakesSettingsField] = app + } + if sidecar != nil { + sideFile[TakesSettingsField] = sidecar + } + return Manifest{Module: "search", Resources: []map[string]any{ + appFile, sideFile, + {"id": "env", "type": "file", "path": "/etc/search.env", "content": "SITE=${setting:site-name}\n"}, + }} +} + +func settledContent(t *testing.T, resource map[string]any, layers []Layer) map[string]any { + t.Helper() + out, err := ApplySettings(resource, layers) + if err != nil { + t.Fatal(err) + } + if _, kept := out[TakesSettingsField]; kept { + t.Errorf("%v still carries %s; the host knows no such field", out["id"], TakesSettingsField) + } + var parsed map[string]any + if err := json.Unmarshal([]byte(out["content"].(string)), &parsed); err != nil { + t.Fatalf("%v is not JSON after settling: %v", out["id"], err) + } + return parsed +} + +func TestEachMergeableFileTakesOnlyTheKeysItNames(t *testing.T) { + // novox/hq ADR 0326 §1, issue 168: two configurable files, each taking its own keys. A key for + // the other file, for a ${setting:…} elsewhere or for the mesh itself lands in neither. + m := twoFiles([]any{"server", "search"}, []any{"url"}) + layers := []Layer{ + {From: "the mesh", Values: map[string]any{ + "server": map[string]any{"base_url": "https://search.example.tld/"}, + "url": "http://127.0.0.1:9090", "site-name": "Search", + "endpoints": map[string]any{"web": map[string]any{"reach": "public"}}}}, + {From: "node-a", Values: map[string]any{"search": map[string]any{"safe": 0.0}}}, + } + + app := settledContent(t, m.Resources[0], layers) + for _, stray := range []string{"url", "site-name", "endpoints"} { + if _, in := app[stray]; in { + t.Errorf("app took %q, which it does not name: %v", stray, app) + } + } + server, _ := app["server"].(map[string]any) + if server["base_url"] != "https://search.example.tld/" || server["port"] != 8080.0 { + t.Errorf("app's server block is %v; the setting should merge into the file's own", server) + } + if search, _ := app["search"].(map[string]any); search["safe"] != 0.0 { + t.Errorf("app's search block is %v; the node's layer should win", search) + } + + side := settledContent(t, m.Resources[1], layers) + if side["url"] != "http://127.0.0.1:9090" { + t.Errorf("sidecar's url is %v; it names url", side["url"]) + } + if len(side) != 1 { + t.Errorf("sidecar took keys it does not name: %v", side) + } +} + +func TestAnEmptyListTakesNoSetting(t *testing.T) { + // ADR 0164 §12: a file that names no setting takes none. + m := twoFiles([]any{"server"}, []any{}) + side := settledContent(t, m.Resources[1], []Layer{{From: "the mesh", Values: map[string]any{ + "server": map[string]any{"port": 1.0}, "url": "x"}}}) + if len(side) != 1 || side["url"] != "http://127.0.0.1:8080" { + t.Errorf("a file taking none was changed: %v", side) + } +} + +func TestAFileThatNamesNothingKeepsTodaysForm(t *testing.T) { + // ADR 0326 §3, ADR 0164 §15: a module none of whose files names keys is left as it was. + m := twoFiles(nil, nil) + app := settledContent(t, m.Resources[0], []Layer{{From: "the mesh", Values: map[string]any{"url": "x"}}}) + if app["url"] != "x" { + t.Errorf("a silent file should still take every key until its module names them: %v", app) + } + if unused := UnusedSettings(m, []Layer{{From: "the mesh", Values: map[string]any{"anything": 1}}}); len(unused) != 0 { + t.Errorf("today's form has nothing stray, and said: %v", unused) + } +} + +func TestAKeyNoFileNamesIsStray(t *testing.T) { + // ADR 0326 §5: once the files name their keys, a key none of them names — and nothing else + // takes — is stray, and is said with what each file takes. + m := twoFiles([]any{"server", "search"}, []any{"url"}) + unused := UnusedSettings(m, []Layer{{From: "the mesh", Values: map[string]any{ + "server": 1, "url": "x", "site-name": "Search", "ui": map[string]any{"theme": "dark"}}}}) + if len(unused) != 1 || !strings.Contains(unused[0], `"ui"`) { + t.Fatalf("only ui reaches nothing; named: %v", unused) + } + if !strings.Contains(unused[0], "app takes server, search") || !strings.Contains(unused[0], "sidecar takes url") { + t.Errorf("the stray key's message does not say what each file takes: %s", unused[0]) + } +} + +func TestOneNamingFileMakesEveryMergeableFileName(t *testing.T) { + // ADR 0326 §2: a module with one naming and one silent mergeable file is refused, naming the + // silent one — the silent one would otherwise still take every key. + m := twoFiles([]any{"server"}, nil) + problems := TakenSettingsProblems(m) + if len(problems) != 1 || !strings.Contains(problems[0], "sidecar merges and names no takes-settings") { + t.Fatalf("problems: %v", problems) + } + if err := JudgeSettings(m, nil, false); err == nil || !strings.Contains(err.Error(), "sidecar") { + t.Errorf("composing it should refuse it too, and said: %v", err) + } + raw, err := json.Marshal(map[string]any{"module": m.Module, "version": "1", "resources": m.Resources}) + if err != nil { + t.Fatal(err) + } + if _, err := ParseManifest(raw); err == nil || !strings.Contains(err.Error(), "sidecar merges and names no") { + t.Errorf("the module check does not say it: %v", err) + } +} + +func TestTakesSettingsIsRefusedWhereItCannotMeanAnything(t *testing.T) { + // ADR 0326 §4. + cases := map[string]struct { + resource map[string]any + says string + }{ + "on a file that does not merge": { + map[string]any{"id": "plain", "type": "file", "path": "/x", "content": "x", + TakesSettingsField: []any{"a"}}, + "does not merge"}, + "not a list": { + map[string]any{"id": "conf", "type": "file", "path": "/x", "merge": MergeJSON, "content": "{}", + TakesSettingsField: "server"}, + "is a list of setting keys"}, + "a mesh word": { + map[string]any{"id": "conf", "type": "file", "path": "/x", "merge": MergeJSON, "content": "{}", + TakesSettingsField: []any{"endpoints"}}, + "the mesh reads"}, + "a protected key": { + map[string]any{"id": "conf", "type": "file", "path": "/x", "merge": MergeJSON, "content": "{}", + "protected": []any{"secret"}, TakesSettingsField: []any{"secret"}}, + "keeps it as protected"}, + "named twice": { + map[string]any{"id": "conf", "type": "file", "path": "/x", "merge": MergeJSON, "content": "{}", + TakesSettingsField: []any{"a", "a"}}, + "twice"}, + "not a key": { + map[string]any{"id": "conf", "type": "file", "path": "/x", "merge": MergeJSON, "content": "{}", + TakesSettingsField: []any{"two words"}}, + "not a usable setting key"}, + } + for name, c := range cases { + problems := TakenSettingsProblems(Manifest{Module: "m", Resources: []map[string]any{c.resource}}) + if !containsText(problems, c.says) { + t.Errorf("%s: want a problem saying %q, got %v", name, c.says, problems) + } + } + // And a file that does not merge is refused where it is settled, not handed to the host. + plain := map[string]any{"id": "plain", "type": "file", "path": "/x", "content": "x", + TakesSettingsField: []any{"a"}} + if _, err := ApplySettings(plain, nil); err == nil { + t.Error("a file that does not merge and names keys was settled") + } +} + +func TestASoftwareSpelledKeyIsUsable(t *testing.T) { + // A file's keys are its software's: nodered's timeZone is not lower case and is still a key. + m := Manifest{Module: "flows", Resources: []map[string]any{{"id": "settings", "type": "file", + "path": "/x", "merge": MergeJSON, "content": "{}", TakesSettingsField: []any{"timeZone"}}}} + if problems := TakenSettingsProblems(m); len(problems) != 0 { + t.Errorf("timeZone was refused: %v", problems) + } +} + +func containsText(list []string, text string) bool { + for _, s := range list { + if strings.Contains(s, text) { + return true + } + } + return false +} + +func TestAModuleThatDeclaresASettingNamesItsFilesKeys(t *testing.T) { + // ADR 0326 §3, ADR 0164 §15: only a module that declares no settings keeps today's form. One that + // declares a preference and owns a silent mergeable file is refused where it is checked — and not + // where a stored definition is composed, so a machine does not lose it before its next change. + m := twoFiles(nil, nil) + m.Settings = map[string]SettingDeclaration{"site-name": {Kind: KindPreference, Default: "Search", Why: "a name"}} + problems := TakenSettingsProblems(m) + if len(problems) != 2 || !containsText(problems, "app names no takes-settings") || + !containsText(problems, "sidecar names no takes-settings") { + t.Errorf("problems: %v", problems) + } + if err := JudgeSettings(m, nil, false); err != nil { + t.Errorf("a stored definition is composed as before, and was refused: %v", err) + } + m = twoFiles([]any{"server"}, []any{}) + m.Settings = map[string]SettingDeclaration{"site-name": {Kind: KindPreference, Default: "Search", Why: "a name"}} + if problems := TakenSettingsProblems(m); len(problems) != 0 { + t.Errorf("every mergeable file names its keys, and was refused: %v", problems) + } +} diff --git a/internal/catalogue/settings.go b/internal/catalogue/settings.go index 534c7424..71357453 100644 --- a/internal/catalogue/settings.go +++ b/internal/catalogue/settings.go @@ -46,11 +46,24 @@ type Layer struct { // rather than by repeating everything the rest already say. func ApplySettings(resource map[string]any, layers []Layer) (map[string]any, error) { how, _ := resource["merge"].(string) + taken, names := takenKeys(resource) if how == "" { + if names { + // Refused by the module check too (TakenSettingsProblems); said here as well, for a + // definition stored by an older binary, rather than handed to the host, which knows no + // such field. + return nil, fmt.Errorf("%v says %s and does not merge: only a mergeable file takes "+ + "settings by key (novox/hq ADR 0326)", resource["id"], TakesSettingsField) + } // Not a mergeable file. Settings for it are ignored rather than silently doing nothing // somewhere else — see UnusedSettings, which is what says so. return resource, nil } + if names { + // **The file takes the keys it names, and no other** (novox/hq ADR 0326, issue 168): a key + // for another file, for a `${setting:}` or for the mesh itself does not land here. + layers = onlyTaken(layers, taken) + } if how != MergeJSON { return nil, fmt.Errorf( "%v says it merges as %q, and this control plane knows how to merge %q", @@ -91,6 +104,7 @@ func ApplySettings(resource map[string]any, layers []Layer) (map[string]any, err delete(out, "merge") delete(out, "protected") delete(out, TrustedField) + delete(out, TakesSettingsField) return out, nil } @@ -324,8 +338,11 @@ func sortedKeysAny(m map[string]any) []string { // (novox/hq 04-ISSUES/173); and the mesh's own words — `expose`, `ports`, `reach`, `endpoints` — // are read by the mesh. A key none of those takes is stray, and is said so rather than dropped. func UnusedSettings(m Manifest, layers []Layer) []string { + named := namesItsKeys(m) for _, r := range m.Resources { - if how, _ := r["merge"].(string); how != "" { + if how, _ := r["merge"].(string); how != "" && !named { + // Today's form (novox/hq ADR 0326 §3, ADR 0164 §15): a mergeable file that names no + // keys takes every key, so nothing is stray. return nil } } @@ -337,6 +354,13 @@ func UnusedSettings(m Manifest, layers []Layer) []string { } lands := settingKeysUsedBy(m) + // A mergeable file takes the keys it names (novox/hq ADR 0326 §5). + for _, r := range m.Resources { + taken, _ := takenKeys(r) + for key := range taken { + lands[key] = true + } + } // A key the module gives a default reaches what asks for it (novox/hq ADR 0262): setting it // overrides the default, which is how a running module gains a setting without a gap between. for key := range m.Settings { @@ -401,8 +425,8 @@ func UnusedSettings(m Manifest, layers []Layer) []string { } unused = append(unused, fmt.Sprintf( "%s sets %q, and %s has no file that merges it, asks for no ${setting:%s}, and "+ - "declares no %q in what it contributes or serves", - layer.From, key, m.Module, key, key)) + "declares no %q in what it contributes or serves%s", + layer.From, key, m.Module, key, key, takenSaid(m))) } } sort.Strings(unused) @@ -512,6 +536,11 @@ func JudgeSettings(m Manifest, layers []Layer, adopted bool) error { if err := settingsHoldOneLine(m.Module, layers); err != nil { return err } + // Which keys each mergeable file takes is the definition's to say right (novox/hq ADR 0326); + // judged here too for a definition stored by an older binary. + if problems := takenSettingsProblems(m); len(problems) > 0 { + return fmt.Errorf("%s", problems[0]) + } // With no layers too: a definition may ask for a setting nobody made — an access placed by // nobody, a file's ${setting:…} nothing sets — and that is the same statement, missing. if _, err := GivenPorts(m, layers); err != nil { From dd108892097101cf5a256b650e8caf1ab50e5dd0 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 18:00:33 +0200 Subject: [PATCH 2/3] Hold back refusing a declaring module's silent mergeable file (hq issue 168) betterbird declares a preference and owns a mergeable file that names no keys, and the catalogue can only name them once this controller runs: the refusal made the catalogue check fail against catalogue main. It follows once mesh-catalog names the keys. --- internal/catalogue/setting_takes.go | 23 ++++++----------------- internal/catalogue/setting_takes_test.go | 21 --------------------- 2 files changed, 6 insertions(+), 38 deletions(-) diff --git a/internal/catalogue/setting_takes.go b/internal/catalogue/setting_takes.go index 6215bd33..0303bc31 100644 --- a/internal/catalogue/setting_takes.go +++ b/internal/catalogue/setting_takes.go @@ -70,25 +70,14 @@ func namesItsKeys(m Manifest) bool { // reads itself, one the file keeps for itself, or one mergeable file naming its keys beside another // that names none. func TakenSettingsProblems(m Manifest) []string { - problems := takenSettingsProblems(m) - // **A module that declares a setting names the keys of every mergeable file** (ADR 0326 §3, ADR - // 0164 §11 and §15): only a module that declares none keeps today's form. Said where a definition - // is checked and registered, never where a stored one is composed: a definition stored before - // this record is brought into the form by its next change, not left out of a machine meanwhile. - if len(m.Settings) > 0 { - for _, r := range m.Resources { - how, _ := r["merge"].(string) - if _, said := r[TakesSettingsField]; how != "" && !said { - problems = append(problems, fmt.Sprintf("%s declares settings, and its mergeable %v names no %s: "+ - "a module that declares a setting names the keys each mergeable file takes, an empty list "+ - "for none (novox/hq ADR 0326, ADR 0164 §11)", m.Module, r["id"], TakesSettingsField)) - } - } - } - return problems + // Not yet here: refusing a module that declares a setting and owns a mergeable file naming no keys + // (ADR 0326 §3). betterbird is one, and the catalogue cannot name its keys before this controller + // runs, so that refusal follows once the catalogue does (novox/hq issue 168's follow-up). + return takenSettingsProblems(m) } -// takenSettingsProblems is what TakenSettingsProblems says that a stored definition is judged by too. +// takenSettingsProblems is every rule of ADR 0326 §2 and §4, judged where a definition is checked and +// where a stored one is composed. func takenSettingsProblems(m Manifest) []string { var problems []string var naming, silent []string diff --git a/internal/catalogue/setting_takes_test.go b/internal/catalogue/setting_takes_test.go index fc59f79b..06ddc3a3 100644 --- a/internal/catalogue/setting_takes_test.go +++ b/internal/catalogue/setting_takes_test.go @@ -194,24 +194,3 @@ func containsText(list []string, text string) bool { } return false } - -func TestAModuleThatDeclaresASettingNamesItsFilesKeys(t *testing.T) { - // ADR 0326 §3, ADR 0164 §15: only a module that declares no settings keeps today's form. One that - // declares a preference and owns a silent mergeable file is refused where it is checked — and not - // where a stored definition is composed, so a machine does not lose it before its next change. - m := twoFiles(nil, nil) - m.Settings = map[string]SettingDeclaration{"site-name": {Kind: KindPreference, Default: "Search", Why: "a name"}} - problems := TakenSettingsProblems(m) - if len(problems) != 2 || !containsText(problems, "app names no takes-settings") || - !containsText(problems, "sidecar names no takes-settings") { - t.Errorf("problems: %v", problems) - } - if err := JudgeSettings(m, nil, false); err != nil { - t.Errorf("a stored definition is composed as before, and was refused: %v", err) - } - m = twoFiles([]any{"server"}, []any{}) - m.Settings = map[string]SettingDeclaration{"site-name": {Kind: KindPreference, Default: "Search", Why: "a name"}} - if problems := TakenSettingsProblems(m); len(problems) != 0 { - t.Errorf("every mergeable file names its keys, and was refused: %v", problems) - } -} From b5383970550b51403824a0e8200ca5a50003dc0c Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 12 Oct 2026 18:00:49 +0200 Subject: [PATCH 3/3] Name the issue that holds the refusal back (hq issue 574) --- internal/catalogue/setting_takes.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/catalogue/setting_takes.go b/internal/catalogue/setting_takes.go index 0303bc31..bfc83c57 100644 --- a/internal/catalogue/setting_takes.go +++ b/internal/catalogue/setting_takes.go @@ -72,7 +72,7 @@ func namesItsKeys(m Manifest) bool { func TakenSettingsProblems(m Manifest) []string { // Not yet here: refusing a module that declares a setting and owns a mergeable file naming no keys // (ADR 0326 §3). betterbird is one, and the catalogue cannot name its keys before this controller - // runs, so that refusal follows once the catalogue does (novox/hq issue 168's follow-up). + // runs, so that refusal follows once the catalogue does (novox/hq issue 574). return takenSettingsProblems(m) }