From b5a441a9784682f75b24f130c9e8a8f664ad4e78 Mon Sep 17 00:00:00 2001 From: jochen Date: Thu, 15 Oct 2026 17:18:23 +0200 Subject: [PATCH] Let a module-declared seat carry needs, so mail-notifier can need node-notifier (issue 802) ADR 0220's dependency mechanism served compiled seats only and retired its one user with node-resolver-config (commit f68521da). ADR 0378 S8 revives it for module-declared seats: SeatDeclaration gains Needs, judged by the catalogue check (node scope only, a name that resolves to a node seat somewhere in the catalogue, never itself), and DependsOn now takes the catalogue so a claimed seat's needs are found wherever they are declared. --- .../contribution_dependencies_test.go | 4 +- internal/catalogue/data_test.go | 6 +- .../resolver_file_is_the_uplinks_test.go | 2 +- internal/catalogue/seat_contributions_test.go | 2 +- internal/catalogue/seat_data_test.go | 4 +- internal/catalogue/seat_dependencies.go | 49 +++++++++++---- internal/catalogue/seat_dependencies_test.go | 36 ++++++++++- internal/catalogue/seats_declared.go | 61 +++++++++++++++++++ internal/catalogue/seats_declared_test.go | 47 ++++++++++++++ 9 files changed, 188 insertions(+), 23 deletions(-) diff --git a/internal/catalogue/contribution_dependencies_test.go b/internal/catalogue/contribution_dependencies_test.go index e56c9579..91e9bc03 100644 --- a/internal/catalogue/contribution_dependencies_test.go +++ b/internal/catalogue/contribution_dependencies_test.go @@ -34,7 +34,7 @@ func TestAContributionDependsOnTheSeatThatPlacesIt(t *testing.T) { "an empty environment": {empty, nil}, } for name, c := range cases { - got := DependsOn(c.m) + got := DependsOn(nil, c.m) if len(got) == 0 { got = nil } @@ -118,7 +118,7 @@ func TestCodeForAPowerMomentDependsOnThePowerSeat(t *testing.T) { for _, moment := range powerMoments { m := mod("laptop", nil, nil, nil) m.Shell = []ShellCode{{For: moment, Slot: "normal", Code: "true"}} - if got := DependsOn(m); !reflect.DeepEqual(got, []string{PowerSeat}) { + if got := DependsOn(nil, m); !reflect.DeepEqual(got, []string{PowerSeat}) { t.Errorf("code for %s depends on %v, want %s", moment, got, PowerSeat) } if p := m.shellProblems(); len(p) != 0 { diff --git a/internal/catalogue/data_test.go b/internal/catalogue/data_test.go index fde16ca2..1f379cb1 100644 --- a/internal/catalogue/data_test.go +++ b/internal/catalogue/data_test.go @@ -197,9 +197,9 @@ func TestTheBackupHoldersLinesAreDerivedFromTheData(t *testing.T) { } // What keeps something irreplaceable depends on the machine's backup holder — it backs it up or // watches its array; valuable data alone is backed up where a holder is, and refused nowhere. - if !slices.Contains(DependsOn(media), BackupSeat) || !slices.Contains(DependsOn(pg), BackupSeat) || - slices.Contains(DependsOn(agent), BackupSeat) { - t.Fatalf("depends: media %v, pg %v, agent %v", DependsOn(media), DependsOn(pg), DependsOn(agent)) + if !slices.Contains(DependsOn(nil, media), BackupSeat) || !slices.Contains(DependsOn(nil, pg), BackupSeat) || + slices.Contains(DependsOn(nil, agent), BackupSeat) { + t.Fatalf("depends: media %v, pg %v, agent %v", DependsOn(nil, media), DependsOn(nil, pg), DependsOn(nil, agent)) } if it, _ := media.DataItem("films"); it.OwnedByModule() || it.Protection() != "redundancy" { t.Fatalf("an operator's path reads %+v", it) diff --git a/internal/catalogue/resolver_file_is_the_uplinks_test.go b/internal/catalogue/resolver_file_is_the_uplinks_test.go index 768d37eb..dc2de447 100644 --- a/internal/catalogue/resolver_file_is_the_uplinks_test.go +++ b/internal/catalogue/resolver_file_is_the_uplinks_test.go @@ -14,7 +14,7 @@ func TestTheResolverConfigSeatIsGone(t *testing.T) { t.Error("node-resolver-config is still in the mesh's set; the uplink's holder writes the resolver file") } // An uplink's holder needs no seat beside it for the file: it is its own. - if got := DependsOn(mod("networkmanager", nil, nil, nil, Claim{Name: "node-uplink"})); len(got) != 0 { + if got := DependsOn(nil, mod("networkmanager", nil, nil, nil, Claim{Name: "node-uplink"})); len(got) != 0 { t.Errorf("an uplink holder with nothing declared depends on %v", got) } } diff --git a/internal/catalogue/seat_contributions_test.go b/internal/catalogue/seat_contributions_test.go index 303e40ba..ad6d31e2 100644 --- a/internal/catalogue/seat_contributions_test.go +++ b/internal/catalogue/seat_contributions_test.go @@ -95,7 +95,7 @@ func TestAContributionDependsOnItsSeat(t *testing.T) { {Seat: HotkeysSeat, Kind: "trigger", Content: "x"}, {Seat: DisplaySessionSeat, Kind: "config", Content: "y"}, }} - if got, want := DependsOn(m), []string{DisplaySessionSeat, HotkeysSeat}; !reflect.DeepEqual(got, want) { + if got, want := DependsOn(nil, m), []string{DisplaySessionSeat, HotkeysSeat}; !reflect.DeepEqual(got, want) { t.Errorf("depends on %v, want %v", got, want) } catalogue := map[string]Manifest{"laptop": m, "triggerhappy": hotkeysHolder()} diff --git a/internal/catalogue/seat_data_test.go b/internal/catalogue/seat_data_test.go index de83c37b..3389ede1 100644 --- a/internal/catalogue/seat_data_test.go +++ b/internal/catalogue/seat_data_test.go @@ -333,9 +333,9 @@ func TestAnOfferedBlockMakesNoDependencyOnTheBar(t *testing.T) { block(`{"bar":"bottom","place":"status","shows":"battery"}`), {Seat: HotkeysSeat, Kind: "trigger", Content: "x"}, }} - for _, seat := range DependsOn(power) { + for _, seat := range DependsOn(nil, power) { if seat == BarSeat { - t.Fatalf("a block made the power module depend on the bar: %v", DependsOn(power)) + t.Fatalf("a block made the power module depend on the bar: %v", DependsOn(nil, power)) } } // And on a machine with no bar it is placed nowhere, and nothing fails. diff --git a/internal/catalogue/seat_dependencies.go b/internal/catalogue/seat_dependencies.go index 08e30a3e..bde015a9 100644 --- a/internal/catalogue/seat_dependencies.go +++ b/internal/catalogue/seat_dependencies.go @@ -6,10 +6,12 @@ import ( "strings" ) -// 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). A third source — what a seat it holds needs beside it -// (novox/hq ADR 0220) — had one user, node-resolver-config needing node-uplink, and went with that -// seat when the resolver file became the uplink's own (novox/hq ADR 0223). +// A module depends on the node seats that apply its resources (novox/hq ADR 0207), on the +// seats it contributes to (novox/hq ADR 0210), and on the seats a seat it holds needs beside it +// (novox/hq ADR 0220). The third source once served only compiled seats and had one user, +// node-resolver-config needing node-uplink, which went with that seat when the resolver file became +// the uplink's own (novox/hq ADR 0223) — and now serves module-declared seats instead: mail-notifier +// needing node-notifier, and the like (novox/hq ADR 0378 §8). // // 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 @@ -76,13 +78,16 @@ func isFoundation(m Manifest) bool { return foundationModules[m.Module] || m.Computed != "" } -// DependsOn is every seat a module needs held on its node, derived from the resource types it -// declares itself (novox/hq ADR 0207 §2), sorted. +// DependsOn is every seat a module needs held on its node: derived from the resource types it +// declares itself (novox/hq ADR 0207 §2), from what it contributes to (ADR 0210), and from what a +// node seat it claims needs beside it — read from whichever manifest in `catalogue` declares that +// seat (novox/hq ADR 0220, ADR 0378 §8). Sorted. // -// **The module's own `resources` only.** What the controller composes around a module — its -// filter, jails, certificates, kept files, bundles, the guard — is the mesh's, put there because the -// module is assigned, and depending on it would make the module answer for the mesh's choices. -func DependsOn(m Manifest) []string { +// **The module's own `resources` only**, for the first source. What the controller composes around +// a module — its filter, jails, certificates, kept files, bundles, the guard — is the mesh's, put +// there because the module is assigned, and depending on it would make the module answer for the +// mesh's choices. +func DependsOn(catalogue map[string]Manifest, m Manifest) []string { if isFoundation(m) { return nil } @@ -95,6 +100,9 @@ func DependsOn(m Manifest) []string { for _, seat := range contributedTo(m) { seen[seat] = true } + for _, seat := range neededBesideClaims(catalogue, m) { + seen[seat] = true + } out := make([]string, 0, len(seen)) for s := range seen { out = append(out, s) @@ -135,6 +143,25 @@ func contributedTo(m Manifest) []string { return out } +// neededBesideClaims is every seat a node seat the module claims needs held on the same node +// (novox/hq ADR 0220, ADR 0378 §8): a module-declared seat's needs are read from whichever manifest +// in the catalogue declares that seat, never written again in the claimant's own manifest. Derived +// from the claim, as a resource's seat is derived from its type, so a module that claims the seat +// cannot leave the dependency out. A compiled seat carries no needs (novox/hq ADR 0378 §8). +func neededBesideClaims(catalogue map[string]Manifest, m Manifest) []string { + var out []string + for _, seat := range nodeSeatsClaimed(m) { + for _, candidate := range catalogue { + for _, s := range candidate.DefinesSeats { + if s.Name == seat { + out = append(out, s.Needs...) + } + } + } + } + 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 { @@ -205,7 +232,7 @@ func UnheldDependencies(catalogue map[string]Manifest, node string, set []Manife if judged != nil && !judged[m.Module] { continue } - for _, seat := range DependsOn(m) { + for _, seat := range DependsOn(catalogue, m) { if held[seat] { continue } diff --git a/internal/catalogue/seat_dependencies_test.go b/internal/catalogue/seat_dependencies_test.go index fc76f9c6..7c717d63 100644 --- a/internal/catalogue/seat_dependencies_test.go +++ b/internal/catalogue/seat_dependencies_test.go @@ -51,7 +51,7 @@ func TestADependencyIsDerivedFromEachResourceTypeThatAppliesThroughASeat(t *test "action": nil, "network": nil, "access": nil, } for kind, want := range cases { - got := DependsOn(withResources(mod("m", nil, nil, nil), res(kind, "x"))) + got := DependsOn(nil, withResources(mod("m", nil, nil, nil), res(kind, "x"))) if len(got) == 0 { got = nil } @@ -59,7 +59,7 @@ func TestADependencyIsDerivedFromEachResourceTypeThatAppliesThroughASeat(t *test t.Errorf("a %s resource depends on %v, want %v", kind, got, want) } } - all := DependsOn(withResources(mod("m", nil, nil, nil), + all := DependsOn(nil, withResources(mod("m", nil, nil, nil), res("container", "a"), res("service", "b"), res("package", "c"), res("package", "d"))) if want := []string{ContainerRuntimeSeat, PackageManagerSeat, ServiceManagerSeat}; !reflect.DeepEqual(all, want) { t.Errorf("a module of every kind depends on %v, want each seat once: %v", all, want) @@ -207,7 +207,7 @@ func TestTheFoundationsDeclarationsAreNotJudged(t *testing.T) { cat := shelf(append(coreThree(), network, host)...) for _, m := range []Manifest{network, host} { - if d := DependsOn(m); len(d) != 0 { + if d := DependsOn(cat, m); len(d) != 0 { t.Errorf("%s, the foundation's, depends on %v", m.Module, d) } } @@ -223,6 +223,36 @@ func TestTheFoundationsDeclarationsAreNotJudged(t *testing.T) { } } +// A module-declared seat's needs are judged exactly as a compiled seat's once were (novox/hq ADR +// 0220, ADR 0378 §8): "mail" declares the node seat "mail-notifier" needing node-notifier, the +// mesh's own (desktop notifications); "notifier" claims mail-notifier; "dunst" claims node-notifier, +// as it does in the real catalogue. +func mailNotifierAndDunst() map[string]Manifest { + return shelf( + Manifest{Module: "mail", DefinesSeats: []SeatDeclaration{{ + Name: "mail-notifier", Scope: ScopeNode, Needs: []string{NotifierSeat}, + }}}, + mod("notifier", nil, nil, nil, Claim{Name: "mail-notifier"}), + mod("dunst", nil, nil, nil, Claim{Name: NotifierSeat}), + ) +} + +func TestAModuleDeclaredSeatsNeedsAreJudgedLikeACompiledSeatsOnceWere(t *testing.T) { + cat := mailNotifierAndDunst() + _, err := AssignRefusal(cat, "workstation", nil, []string{"notifier"}) + var refusal *Refusal + if !errors.As(err, &refusal) || !strings.Contains(err.Error(), "node-notifier") { + t.Fatalf("notifier assigned without dunst was not refused naming node-notifier: %v", err) + } + if _, err := AssignRefusal(cat, "workstation", []string{"dunst"}, []string{"notifier"}); err != nil { + t.Errorf("notifier beside dunst was refused: %v", err) + } + if err := UnassignRefusal(cat, "workstation", []string{"dunst", "notifier"}, []string{"dunst"}); err == nil || + !strings.Contains(err.Error(), "node-notifier") { + t.Errorf("taking dunst from under notifier was not refused: %v", err) + } +} + func TestUnassigningTheLastHolderWhileDependentsRemainIsRefused(t *testing.T) { sshd := withResources(mod("sshd", nil, nil, nil), res("service", "sshd")) cat := shelf(append(coreThree(), sshd)...) diff --git a/internal/catalogue/seats_declared.go b/internal/catalogue/seats_declared.go index 77c64810..d53d3e04 100644 --- a/internal/catalogue/seats_declared.go +++ b/internal/catalogue/seats_declared.go @@ -71,6 +71,12 @@ type SeatDeclaration struct { // mesh's own seats name theirs by: a node seat only, each absolute and literal, and none another seat // names. A module's seat never steps back. Paths []string `json:"paths,omitempty"` + // Needs is every node seat this seat's holder needs held on its own node (novox/hq ADR 0220, ADR + // 0378 §8): a role whose holder is only right while another role is filled beside it. A module + // claiming this seat depends on each, exactly as a module declaring a service depends on the + // service manager (ADR 0207) — derived from the claim, never written again in the claimant's own + // manifest. Only a node seat may carry needs, and a compiled seat carries none. + Needs []string `json:"needs,omitempty"` } // KindedBenches are the seats that may be kinded (novox/hq ADR 0234 §2): `channel` sends to the operator, @@ -168,6 +174,7 @@ func declaredSeatProblems(m Manifest) []string { } problems = append(problems, trafficProblems(m, s)...) problems = append(problems, declaredSeatPathProblems(m, s)...) + problems = append(problems, declaredSeatNeedsProblems(m, s)...) } for _, u := range m.Uses { @@ -228,6 +235,30 @@ func trafficProblems(m Manifest, s SeatDeclaration) []string { return problems } +// declaredSeatNeedsProblems is what one declaration's needs can be judged on alone (novox/hq ADR +// 0378 §8): only a node seat is ever held beside another on the same node, so only a node seat may +// carry needs; and a seat needing itself names nothing that was not already true of holding it. +// Whether each named need itself exists as a node seat somewhere in the catalogue is CatalogueProblems' +// to say, once every manifest is known. +func declaredSeatNeedsProblems(m Manifest, s SeatDeclaration) []string { + var problems []string + if len(s.Needs) == 0 { + return problems + } + if s.At() != ScopeNode { + problems = append(problems, fmt.Sprintf( + "%s declares %s needing %s, and only a node seat needs another held beside it "+ + "(novox/hq ADR 0378 §8)", m.Module, s.Name, strings.Join(s.Needs, ", "))) + } + for _, need := range s.Needs { + if need == s.Name { + problems = append(problems, fmt.Sprintf( + "%s declares %s needing itself (novox/hq ADR 0378 §8)", m.Module, s.Name)) + } + } + return problems +} + // A Shelf is every manifest the mesh has registered, by module name. type Shelf map[string]Manifest @@ -280,6 +311,36 @@ func CatalogueProblems(shelf Shelf) []string { return ok } + // atNode says whether a seat, compiled or declared, is a node seat — judged here because only + // the whole catalogue knows a declared one's scope (novox/hq ADR 0378 §8). + atNode := func(seat string) bool { + if s, isMesh := SeatNamed(seat); isMesh { + return s.Scope == ScopeNode + } + if s, ok := declared[seat]; ok { + return s.At() == ScopeNode + } + return false + } + + // A module-declared seat's needs name node seats that hold beside it (novox/hq ADR 0378 §8): + // whether one exists at all, compiled or declared by any module, only the whole catalogue can + // say — a single manifest's own problems (declaredSeatProblems) judge its needs no further. + for _, module := range shelfOrder(shelf) { + for _, s := range shelf[module].DefinesSeats { + for _, need := range s.Needs { + if !exists(need) { + problems = append(problems, fmt.Sprintf( + "%s declares %s needing %s, "+UndeclaredSeat, module, s.Name, need)) + } else if !atNode(need) { + problems = append(problems, fmt.Sprintf( + "%s declares %s needing %s, which is not a node seat (novox/hq ADR 0378 §8)", + module, s.Name, need)) + } + } + } + } + // Which seats name each path (novox/hq ADR 0287): a module's seat naming one another seat names loses. named, pathProblems := pathSeats(shelfOrder(shelf), shelf, declaredBy) problems = append(problems, pathProblems...) diff --git a/internal/catalogue/seats_declared_test.go b/internal/catalogue/seats_declared_test.go index 42958a83..33b3ba6c 100644 --- a/internal/catalogue/seats_declared_test.go +++ b/internal/catalogue/seats_declared_test.go @@ -116,6 +116,53 @@ func TestTheMeshsOwnSeatsAreStillClaimable(t *testing.T) { } } +// Only a node seat may carry needs (novox/hq ADR 0378 §8): a per-mesh seat's holder is not on any +// one node to depend on anything beside it there. +func TestOnlyANodeSeatMayCarryNeeds(t *testing.T) { + m := Manifest{Module: "mail", DefinesSeats: []SeatDeclaration{{ + Name: "mail-notifier", Scope: ScopeMesh, Needs: []string{"node-notifier"}}}} + got := strings.Join(declaredSeatProblems(m), "; ") + if !strings.Contains(got, "needing") || !strings.Contains(got, "node seat") { + t.Fatalf("a mesh-scope seat's needs were accepted: %q", got) + } +} + +// A seat needing itself names nothing (novox/hq ADR 0378 §8). +func TestASeatCannotNeedItself(t *testing.T) { + m := Manifest{Module: "mail", DefinesSeats: []SeatDeclaration{{ + Name: "mail-notifier", Scope: ScopeNode, Needs: []string{"mail-notifier"}}}} + got := strings.Join(declaredSeatProblems(m), "; ") + if !strings.Contains(got, "needing itself") { + t.Fatalf("a seat needing itself was accepted: %q", got) + } +} + +// A need naming nothing node-scope — undeclared, or declared at another scope — is the whole +// catalogue's to say (novox/hq ADR 0378 §8). +func TestANeedMustNameANodeSeatSomewhereInTheCatalogue(t *testing.T) { + mail := Manifest{Module: "mail", DefinesSeats: []SeatDeclaration{{ + Name: "mail-notifier", Scope: ScopeNode, Needs: []string{"desk-beeper"}}}} + + // Nobody declares desk-beeper at all. + got := problemsFor(t, Shelf{"mail": mail}) + if !strings.Contains(got, "desk-beeper") || !strings.Contains(got, "no module declares") { + t.Fatalf("needing an undeclared seat was accepted: %q", got) + } + + // Declared, but at mesh scope: still not a node seat to hold beside it. + notNode := Manifest{Module: "dunst", DefinesSeats: []SeatDeclaration{{Name: "desk-beeper", Scope: ScopeMesh}}} + got = problemsFor(t, Shelf{"mail": mail, "dunst": notNode}) + if !strings.Contains(got, "desk-beeper") || !strings.Contains(got, "not a node seat") { + t.Fatalf("needing a mesh-scope seat was accepted: %q", got) + } + + // Declared at node scope: accepted. + dunst := Manifest{Module: "dunst", DefinesSeats: []SeatDeclaration{{Name: "desk-beeper", Scope: ScopeNode}}} + if got := problemsFor(t, Shelf{"mail": mail, "dunst": dunst}); got != "" { + t.Fatalf("needing a node seat another module declares was refused: %q", got) + } +} + // A refusal that reorders itself between runs is a refusal nobody can diff. func TestTheProblemsAreStable(t *testing.T) { shelf := Shelf{"telegram": telegram(), "shop": {Module: "shop", Uses: []string{"nope"}},