From 6af891e3588c95e0d2ab7399d4585a65a191e0ac Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 4 Oct 2026 15:45:35 +0200 Subject: [PATCH] A contribution depends on the seat that receives it, and a collision is refused at assign (hq ADR 0210, issue 235) The environment and shell contributions were written nowhere on a node without their holder; they now derive a dependency on node-environment, node-login-shell or node-display-server, met and refused as ADR 0207's are. Two modules declaring one package, path or unit made the node unresolvable after the assignment was recorded; that is refused first now, because no later assignment can complete it. --- cmd/mesh-controller/acts.go | 5 + .../contribution_dependencies_test.go | 100 ++++++++++++++++++ internal/catalogue/seat_dependencies.go | 74 +++++++++++-- 3 files changed, 170 insertions(+), 9 deletions(-) create mode 100644 internal/catalogue/contribution_dependencies_test.go diff --git a/cmd/mesh-controller/acts.go b/cmd/mesh-controller/acts.go index 45b054a..bcd4626 100644 --- a/cmd/mesh-controller/acts.go +++ b/cmd/mesh-controller/acts.go @@ -160,6 +160,11 @@ func seatDependenciesOnAssign(ctx context.Context, open *stores, node string, mo if _, err := catalogue.AssignRefusal(shelf, node, assigned, adding); err != nil { return nil, nil, err } + // Two modules declaring one package, path or unit is refused before anything is recorded + // (novox/hq ADR 0210, 04-ISSUES/235): kept, the node would not resolve until one came off again. + if err := catalogue.CollisionRefusal(shelf, node, assigned, adding); err != nil { + return nil, nil, err + } return shelf, assigned, nil } diff --git a/internal/catalogue/contribution_dependencies_test.go b/internal/catalogue/contribution_dependencies_test.go new file mode 100644 index 0000000..5c93242 --- /dev/null +++ b/internal/catalogue/contribution_dependencies_test.go @@ -0,0 +1,100 @@ +package catalogue + +import ( + "reflect" + "strings" + "testing" +) + +// Defends novox/hq ADR 0210 §3: a contribution is a dependency on the seat that receives it. + +func TestAContributionDependsOnTheSeatThatPlacesIt(t *testing.T) { + env := mod("theme", nil, nil, nil) + env.Environment = &Environment{Variables: map[string]string{"GTK_THEME": "Adwaita:dark"}} + path := mod("toolchain", nil, nil, nil) + path.Environment = &Environment{Path: []PathEntry{{Entry: "/opt/x/bin"}}} + shell := mod("prompt", nil, nil, nil) + shell.Shell = []ShellCode{{For: "zsh", Slot: "first", Code: "true"}} + session := mod("wallpaper", nil, nil, nil) + session.Shell = []ShellCode{{For: "xinitrc", Slot: "normal", Code: "true"}} + resources := mod("bar", nil, nil, nil) + resources.Shell = []ShellCode{{For: "xresources", Slot: "normal", Code: "x: y"}} + empty := mod("nothing", nil, nil, nil) + empty.Environment = &Environment{} + + cases := map[string]struct { + m Manifest + want []string + }{ + "a variable": {env, []string{EnvironmentSeat}}, + "a path entry": {path, []string{EnvironmentSeat}}, + "shell code": {shell, []string{LoginShellSeat}}, + "the session's start": {session, []string{DisplayServerSeat}}, + "the session's X resources": {resources, []string{DisplayServerSeat}}, + "an empty environment": {empty, nil}, + } + for name, c := range cases { + got := DependsOn(c.m) + if len(got) == 0 { + got = nil + } + if !reflect.DeepEqual(got, c.want) { + t.Errorf("%s depends on %v, want %v", name, got, c.want) + } + } +} + +func TestAContributionIsMetByAHolderOnTheNodeAndRefusedWithout(t *testing.T) { + holder := mod("node-env", nil, nil, nil, Claim{Name: EnvironmentSeat}) + contributor := mod("theme", nil, nil, nil) + contributor.Environment = &Environment{Variables: map[string]string{"GTK_THEME": "Adwaita:dark"}} + catalogue := map[string]Manifest{"node-env": holder, "theme": contributor} + + if _, err := AssignRefusal(catalogue, "laptop", []string{"node-env"}, []string{"theme"}); err != nil { + t.Fatalf("a contributor beside the holder is refused: %v", err) + } + _, err := AssignRefusal(catalogue, "laptop", nil, []string{"theme"}) + if err == nil { + t.Fatal("a contributor on a node without the holder was accepted, and its contribution would be written nowhere") + } + if !strings.Contains(err.Error(), EnvironmentSeat) || !strings.Contains(err.Error(), "node-env") { + t.Errorf("the refusal names neither the seat nor its holder: %v", err) + } + if _, err := AssignRefusal(catalogue, "laptop", nil, []string{"theme", "node-env"}); err != nil { + t.Errorf("the contributor and the holder assigned together are refused: %v", err) + } +} + +func TestAHolderMeetsItsOwnContribution(t *testing.T) { + // The display server's module contributes nothing to its own seat today, but the zsh module's + // environment contributions do go to another seat: a claim meets only the seat it names. + zsh := mod("zsh", nil, nil, nil, Claim{Name: LoginShellSeat}) + zsh.Shell = []ShellCode{{For: "zsh", Slot: "normal", Code: "true"}} + zsh.Environment = &Environment{Variables: map[string]string{"EDITOR": "vim"}} + catalogue := map[string]Manifest{"zsh": zsh, + "node-env": mod("node-env", nil, nil, nil, Claim{Name: EnvironmentSeat})} + unheld := UnheldDependencies(catalogue, "laptop", []Manifest{zsh}, nil) + if len(unheld) != 1 || unheld[0].Seat != EnvironmentSeat { + t.Errorf("zsh alone: unheld %v, want only %s (its shell code is its own seat's)", unheld, EnvironmentSeat) + } +} + +func TestTwoModulesDeclaringOnePackageAreRefusedBeforeAnythingIsRecorded(t *testing.T) { + pacman := withResources(mod("pacman", nil, nil, nil, Claim{Name: PackageManagerSeat}), + res("package", "pacman-contrib")) + bar := withResources(mod("bar", nil, nil, nil), res("package", "bar"), res("package", "pacman-contrib")) + other := withResources(mod("other", nil, nil, nil), res("package", "other")) + catalogue := map[string]Manifest{"pacman": pacman, "bar": bar, "other": other} + + err := CollisionRefusal(catalogue, "laptop", []string{"pacman"}, []string{"bar"}) + if err == nil || !strings.Contains(err.Error(), "pacman-contrib") || !strings.Contains(err.Error(), "bar") { + t.Fatalf("the second owner of a package was not refused by name: %v", err) + } + if err := CollisionRefusal(catalogue, "laptop", []string{"pacman"}, []string{"other"}); err != nil { + t.Errorf("a module declaring nothing shared is refused: %v", err) + } + // A collision already on the node is status's, not a reason to refuse an unrelated assignment. + if err := CollisionRefusal(catalogue, "laptop", []string{"pacman", "bar"}, []string{"other"}); err != nil { + t.Errorf("an unrelated assignment is refused for a collision already there: %v", err) + } +} diff --git a/internal/catalogue/seat_dependencies.go b/internal/catalogue/seat_dependencies.go index 80539b3..dcb7b61 100644 --- a/internal/catalogue/seat_dependencies.go +++ b/internal/catalogue/seat_dependencies.go @@ -6,7 +6,8 @@ import ( "strings" ) -// A module depends on the node seats that apply its resources (novox/hq ADR 0207). +// A module depends on the node seats that apply its resources (novox/hq ADR 0207), and on the +// seats it contributes to (novox/hq ADR 0210). // // Some of what a module declares is applied through software on the machine that is itself a // module: a service through the service manager, a package through the package manager, a container @@ -89,6 +90,9 @@ func DependsOn(m Manifest) []string { seen[seat] = true } } + for _, seat := range contributedTo(m) { + seen[seat] = true + } out := make([]string, 0, len(seen)) for s := range seen { out = append(out, s) @@ -97,6 +101,22 @@ func DependsOn(m Manifest) []string { return out } +// contributedTo is every seat a module contributes to (novox/hq ADR 0210 §3): a contribution is +// configuration only the seat's holder applies, so it is a dependency on that seat exactly as a +// resource is on the seat that applies it. The environment goes to node-environment's holder +// (ADR 0203); shell code to the holder that places it for its target — the login shell's for a +// shell, the display server's for the session's start and resources (ADR 0204, ADR 0208 §4). +func contributedTo(m Manifest) []string { + var out []string + if e := m.Environment; e != nil && (len(e.Variables) > 0 || len(e.Path) > 0) { + out = append(out, EnvironmentSeat) + } + for _, c := range m.Shell { + out = append(out, placerOf(c.For)) + } + return out +} + // claimsSeat is whether a module claims a node seat, by its current name or one it used to have // (ADR 0122), so a rename leaves the dependency met. func claimsSeat(m Manifest, seat string) bool { @@ -158,10 +178,8 @@ func (u Unheld) String() string { func UnheldDependencies(catalogue map[string]Manifest, node string, set []Manifest, judged map[string]bool) []Unheld { held := map[string]bool{} for _, m := range set { - for seat := range seatsApplying() { - if claimsSeat(m, seat) { - held[seat] = true - } + for _, seat := range nodeSeatsClaimed(m) { + held[seat] = true } } var out []Unheld @@ -186,10 +204,19 @@ func UnheldDependencies(catalogue map[string]Manifest, node string, set []Manife return out } -func seatsApplying() map[string]bool { - out := map[string]bool{} - for _, s := range appliedThrough { - out[s] = true +// nodeSeatsClaimed is every node seat a module claims, each by its current name (ADR 0122), so +// a dependency on any of them — a resource's or a contribution's — is met by the claim. +func nodeSeatsClaimed(m Manifest) []string { + var out []string + for _, c := range m.Claims { + if c.At() != ScopeNode { + continue + } + name := c.Name + if s, known := SeatNamed(name); known { + name = s.Name + } + out = append(out, name) } return out } @@ -284,3 +311,32 @@ func UnassignRefusal(catalogue map[string]Manifest, node string, assigned, remov } return &Refusal{Problems: problems} } + +// CollisionRefusal is why assigning `adding` beside `assigned` is refused for what two modules +// would both declare, or nothing (novox/hq ADR 0210 §1, 04-ISSUES/235). +// +// **Refused, not kept like an unresolved provision.** An assignment is otherwise kept when the +// node does not resolve, because assignment is not an ordering: a consumer's provider can follow. +// A collision is not an order anything can complete — no further assignment makes two owners of one +// package one owner — and kept, it leaves the node unresolvable, so the next push of anything drops +// it from the mesh. Only collisions involving a module being added are refused; one already on the +// node is `status`'s, and refusing an unrelated assignment for it would block its own remedy. +func CollisionRefusal(catalogue map[string]Manifest, node string, assigned, adding []string) error { + before := map[string]bool{} + for _, p := range checkResources(manifestsOf(catalogue, assigned)) { + before[p] = true + } + var problems []string + for _, p := range checkResources(manifestsOf(catalogue, append(append([]string(nil), assigned...), adding...))) { + if before[p] { + continue // on the node already; not this assignment's doing + } + problems = append(problems, p+" (novox/hq ADR 0210: one owner per node; the other module "+ + "depends on the owner's seat instead)") + } + if len(problems) == 0 { + return nil + } + sort.Strings(problems) + return &Refusal{Problems: append(problems, fmt.Sprintf("nothing was assigned to %s", node))} +}