From 13b6fc6d97f371504cc6ee8b51b62a52ccf1f69b Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 7 Oct 2026 20:05:01 +0200 Subject: [PATCH] Let failed join the seat optional, and let a seeded row carry both changes failed was required, so the controller refused the systemd module running today and the module serving it was refused by the controller running today: neither could land first. It is now optional (Verb.Optional, #117). Two things kept either change from reaching a mesh whose seat rows already exist: re-seeding added a verb but never an argument to one, and the console refuses an argument the row does not name, so the journal window would stay unreachable; and the optional mark is never stored, so a verb seeded into a row came back required. A row's verb now gains the arguments the binary names, and the working set takes the optional mark from the compiled seat, which also keeps mesh-delivery's checks optional once seeded. --- cmd/mesh-controller/seat_dependencies_test.go | 2 +- internal/catalogue/operators_machine_test.go | 50 +++++++++++++ internal/catalogue/seats.go | 31 +++++++- internal/inventory/seats.go | 49 +++++++++++++ internal/inventory/seats_test.go | 71 +++++++++++++++++++ 5 files changed, 199 insertions(+), 4 deletions(-) diff --git a/cmd/mesh-controller/seat_dependencies_test.go b/cmd/mesh-controller/seat_dependencies_test.go index 4888337..bea72be 100644 --- a/cmd/mesh-controller/seat_dependencies_test.go +++ b/cmd/mesh-controller/seat_dependencies_test.go @@ -15,7 +15,7 @@ import ( func serviceManagerHolder() catalogue.Manifest { return catalogue.Manifest{Module: "systemd", Version: "1", Claims: []catalogue.Claim{{Name: catalogue.ServiceManagerSeat, Scope: catalogue.ScopeNode, - Serves: []string{"units", "status", "start", "stop", "restart", "enable", "disable", "journal", "failed"}}}, + Serves: []string{"units", "status", "start", "stop", "restart", "enable", "disable", "journal"}}}, Resources: []map[string]any{{"id": "systemd", "type": "package", "package": "systemd"}}} } diff --git a/internal/catalogue/operators_machine_test.go b/internal/catalogue/operators_machine_test.go index 40551f0..18fba54 100644 --- a/internal/catalogue/operators_machine_test.go +++ b/internal/catalogue/operators_machine_test.go @@ -58,3 +58,53 @@ func TestTheServiceManagerSeatServesTheUnitVerbs(t *testing.T) { t.Fatalf("the seat serves %v, not %v", got, want) } } + +// **`failed` joins the seat optional** (the operator's direction 2026-10-07): the holder running today, +// which does not serve it, still holds the seat; the holder that serves it is not refused; and a verb +// the seat does not promise is still refused, and one it requires is still required. +func TestFailedIsAnOptionalVerbOfTheServiceManager(t *testing.T) { + seat, _ := SeatNamed(ServiceManagerSeat) + holder := func(serves ...string) Manifest { + return Manifest{Module: "systemd", Version: "1", Claims: []Claim{{Name: ServiceManagerSeat, Scope: ScopeNode, Serves: serves}}} + } + eight := []string{"units", "status", "start", "stop", "restart", "enable", "disable", "journal"} + if err := CanHold(holder(eight...), seat); err != nil { + t.Fatalf("today's holder, without failed, is refused: %v", err) + } + if err := CanHold(holder(append(eight, "failed")...), seat); err != nil { + t.Fatalf("a holder serving failed is refused: %v", err) + } + if err := CanHold(holder(append(eight, "fail")...), seat); err == nil || !strings.Contains(err.Error(), "does not promise") { + t.Fatalf("a verb the seat does not promise was accepted: %v", err) + } + if err := CanHold(holder(eight[1:]...), seat); err == nil || !strings.Contains(err.Error(), "does not serve units") { + t.Fatalf("a holder missing a required verb was accepted: %v", err) + } + for _, v := range seat.Serves { + if v.Optional != (v.Name == "failed") { + t.Errorf("%s optional: %v", v.Name, v.Optional) + } + } +} + +// The optional mark is not stored, so a seat set read back from the store's rows takes it from the +// compiled seat: otherwise `failed`, seeded into the row, would come back required. +func TestAnOptionalVerbStaysOptionalInASetReadFromTheStore(t *testing.T) { + defer UseSeats(DefaultSeats()) + var rows []Seat + for _, s := range DefaultSeats() { + row := s + row.Serves = nil + for _, v := range s.Serves { + row.Serves = append(row.Serves, Verb{Name: v.Name, Description: v.Description, Input: v.Input}) + } + rows = append(rows, row) + } + UseSeats(rows) + seat, _ := SeatNamed(ServiceManagerSeat) + holder := Manifest{Module: "systemd", Version: "1", Claims: []Claim{{Name: ServiceManagerSeat, Scope: ScopeNode, + Serves: []string{"units", "status", "start", "stop", "restart", "enable", "disable", "journal"}}}} + if err := CanHold(holder, seat); err != nil { + t.Fatalf("a set read from rows refuses today's holder: %v", err) + } +} diff --git a/internal/catalogue/seats.go b/internal/catalogue/seats.go index 950afe7..5f05acd 100644 --- a/internal/catalogue/seats.go +++ b/internal/catalogue/seats.go @@ -338,12 +338,35 @@ func UseSeats(s []Seat) { if d, known := byName[row.Name]; known { row.Receives = d.Receives row.Replicated = d.Replicated + row.Serves = optionalAsCompiled(row.Serves, d.Serves) } merged = append(merged, row) } seats = merged } +// optionalAsCompiled is a row's verbs with each one the compiled seat marks optional marked so. The mark +// is never stored (Verb.Optional), and a row is what the working set holds once the store is read: a verb +// the seeding added to the row — `failed`, `checks` — would otherwise come back required, and every holder +// not serving it yet would be refused at registration and at handover, which the mark exists to prevent. +func optionalAsCompiled(row, compiled []Verb) []Verb { + optional := map[string]bool{} + for _, v := range compiled { + if v.Optional { + optional[v.Name] = true + } + } + if len(optional) == 0 { + return row + } + out := make([]Verb, len(row)) + for i, v := range row { + v.Optional = optional[v.Name] + out[i] = v + } + return out +} + // aliases maps a seat's former names to its current canonical name (novox/hq ADR 0122). Loaded from // the store alongside the set, so a reference to a name a seat used to have — a manifest's claim, a // held record — still resolves to it after a rename, and nothing downstream has to change. @@ -562,7 +585,7 @@ func SeatsWithAProtocol() []Seat { // 0177): the units on the machine in both scopes, read and acted on by name. Every verb takes an // optional scope — "system" when absent, "user" for the operator account's own manager — so a // caller asks for a user unit the way it asks for a system one; `failed` alone reads both managers -// when none is named. +// when none is named, and is optional (Verb.Optional). func serviceManagerVerbs() []Verb { scoped := func(more map[string]string, required []string) map[string]any { props := map[string]string{"scope": "\"system\" (the default) or \"user\": the operator account's own manager"} @@ -603,8 +626,10 @@ func serviceManagerVerbs() []Verb { "priority": "only entries this severe or more: 0-7 or emerg, alert, crit, err, warning, notice, info, debug (optional)", }, []string{"unit"})}, // What has failed, on the seat rather than as one holder's own tool: whatever holds the role answers - // it, so a caller asks every machine the same way. - {Name: "failed", Description: "Every failed unit on this machine, in the system manager and in the operator " + + // it, so a caller asks every machine the same way. **Optional while its holders catch up**: the + // systemd module running today serves it as its own systemd_failed, and a required verb would + // refuse it before the version serving `failed` could be delivered. + {Name: "failed", Optional: true, Description: "Every failed unit on this machine, in the system manager and in the operator " + "account's; a manager that does not answer is reported with its error, never as nothing failed.", Input: schema(map[string]string{"scope": "\"system\" or \"user\": only that manager (both when absent)"}, nil)}, } diff --git a/internal/inventory/seats.go b/internal/inventory/seats.go index a0065f3..2c7ceff 100644 --- a/internal/inventory/seats.go +++ b/internal/inventory/seats.go @@ -126,6 +126,16 @@ func (i *Inventory) widenProtocol(ctx context.Context, s catalogue.Seat) error { if s.Name == catalogue.ControllerSeatName && !sameVerb(row.Serves[at], v) { row.Serves[at] = v changed = true + continue + } + // **Any other seat's verb gains the arguments the binary names and the row lacks** — additive, + // as the rest of the protocol is. The console refuses an argument the row does not name (issue + // 244), so a holder taught a new argument (the service manager's journal window) would be + // unreachable through it while the row kept the older schema. Nothing the row has is removed or + // made required; the description is the binary's, since it describes the arguments added. + if widened, ok := widenInput(row.Serves[at], v); ok { + row.Serves[at] = widened + changed = true } } if !changed { @@ -296,6 +306,45 @@ func (i *Inventory) Holdings(ctx context.Context) ([]catalogue.Held, error) { return out, rows.Err() } +// widenInput is the row's verb with every input property the binary's names and the row's lacks, and +// the binary's description; and whether anything was added. +func widenInput(row, binary catalogue.Verb) (catalogue.Verb, bool) { + want, _ := binary.Input["properties"].(map[string]any) + if len(want) == 0 { + return row, false + } + have, _ := row.Input["properties"].(map[string]any) + added := map[string]any{} + for name, p := range want { + if _, kept := have[name]; !kept { + added[name] = p + } + } + if len(added) == 0 { + return row, false + } + input := map[string]any{} + for k, v := range row.Input { + input[k] = v + } + if input["type"] == nil { + input["type"] = "object" + } + props := map[string]any{} + for k, v := range have { + props[k] = v + } + for k, v := range added { + props[k] = v + } + input["properties"] = props + row.Input = input + if binary.Description != "" { + row.Description = binary.Description + } + return row, true +} + // sameVerb is whether two definitions of a verb say the same, read as the row stores them. func sameVerb(a, b catalogue.Verb) bool { ja, errA := json.Marshal(a) diff --git a/internal/inventory/seats_test.go b/internal/inventory/seats_test.go index ab51567..0c1f2a3 100644 --- a/internal/inventory/seats_test.go +++ b/internal/inventory/seats_test.go @@ -156,3 +156,74 @@ func TestTheControllersVerbsInTheRowAreTheBinarys(t *testing.T) { t.Fatal("a verb this binary adds was not added") } } + +// **A seat's verb gains the arguments a newer binary names** (the service manager's journal window, +// 2026-10-07): the console refuses an argument the row does not name, so a row seeded before them would +// keep the holder's new arguments unreachable. Added, never removed — an argument only the row has stays, +// and nothing becomes required — and a verb the binary adds optional is optional in the working set read back. +func TestASeatsVerbGainsTheArgumentsTheBinaryNames(t *testing.T) { + inv := ForTest(t) + ctx := t.Context() + if _, err := inv.SeedSeats(ctx, catalogue.DefaultSeats()); err != nil { + t.Fatal(err) + } + old := `[{"name":"journal","description":"The last lines of one unit's journal.","input":{"type":"object", + "properties":{"unit":{"type":"string"},"lines":{"type":"string"},"scope":{"type":"string"},"kept":{"type":"string"}}, + "required":["unit"]}}]` + if _, err := inv.store.Pool().Exec(ctx, `update seat set serves = $1 where name = $2`, + []byte(old), catalogue.ServiceManagerSeat); err != nil { + t.Fatal(err) + } + if _, err := inv.SeedSeats(ctx, catalogue.DefaultSeats()); err != nil { + t.Fatal(err) + } + seats, err := inv.Seats(ctx) + if err != nil { + t.Fatal(err) + } + verbs := map[string]catalogue.Verb{} + for _, s := range seats { + if s.Name == catalogue.ServiceManagerSeat { + for _, v := range s.Serves { + verbs[v.Name] = v + } + } + } + props, _ := verbs["journal"].Input["properties"].(map[string]any) + for _, arg := range []string{"since", "until", "match", "priority", "unit", "lines", "scope", "kept"} { + if _, has := props[arg]; !has { + t.Errorf("journal in the row does not take %s: %v", arg, props) + } + } + if req, _ := verbs["journal"].Input["required"].([]any); len(req) != 1 || req[0] != "unit" { + t.Errorf("what journal requires changed: %v", verbs["journal"].Input["required"]) + } + if _, added := verbs["failed"]; !added { + t.Fatal("failed was not added to the row") + } + // The optional mark is not stored; the working set read from the rows takes it from the compiled seat, + // so today's holder, which does not serve failed, still holds the seat. + catalogue.UseSeats(seats) + defer catalogue.UseSeats(catalogue.DefaultSeats()) + live, _ := catalogue.SeatNamed(catalogue.ServiceManagerSeat) + holder := catalogue.Manifest{Module: "systemd", Version: "1", Claims: []catalogue.Claim{{Name: catalogue.ServiceManagerSeat, + Scope: catalogue.ScopeNode, Serves: []string{"units", "status", "start", "stop", "restart", "enable", "disable", "journal"}}}} + if err := catalogue.CanHold(holder, live); err != nil { + t.Fatalf("the seat read from the store refuses today's holder: %v", err) + } + // Seeding again changes nothing more. + before := verbs["journal"] + if _, err := inv.SeedSeats(ctx, catalogue.DefaultSeats()); err != nil { + t.Fatal(err) + } + after, _ := inv.Seats(ctx) + for _, s := range after { + if s.Name == catalogue.ServiceManagerSeat { + for _, v := range s.Serves { + if v.Name == "journal" && !sameVerb(v, before) { + t.Fatalf("re-seeding changed journal again: %+v", v) + } + } + } + } +}