From 4771ee53d93881b1b4f32e517ed05c6cc31f5c7d Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 15:21:10 +0200 Subject: [PATCH 1/8] Name the failing line in a failed check's summary, not the warning count printed last (issue 1310) --- internal/builder/check.go | 15 +++++++++++++++ internal/builder/check_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/internal/builder/check.go b/internal/builder/check.go index 3d333a8c..075c55cb 100644 --- a/internal/builder/check.go +++ b/internal/builder/check.go @@ -1773,9 +1773,24 @@ func whatFailed(s string) string { } } } + // The controller's module check refuses with a count line and one line per problem, then prints its + // warning counts: the refusal is what failed, not the count printed last (novox/hq issue 1310). + for i, line := range lines { + if line = strings.TrimSpace(line); problemsLine.MatchString(line) { + if i+1 < len(lines) { + if first := strings.TrimSpace(lines[i+1]); first != "" { + return line + ": " + first + } + } + return line + } + } return lastLine(s) } +// problemsLine is the module check's refusal: "mesh-controller: 3 problem(s) in 139 manifest(s)". +var problemsLine = regexp.MustCompile(`^\S+: [1-9]\d* problem\(s\) in \d+ manifest\(s\)$`) + func lastLine(s string) string { lines := strings.Split(strings.TrimSpace(s), "\n") return strings.TrimSpace(lines[len(lines)-1]) diff --git a/internal/builder/check_test.go b/internal/builder/check_test.go index 63aea2a4..7618a3b1 100644 --- a/internal/builder/check_test.go +++ b/internal/builder/check_test.go @@ -199,3 +199,29 @@ func TestOnlyATrysSelectionReachesTheScript(t *testing.T) { t.Fatalf("a try's selection: %v", env) } } + +// **Issue 1310**: the controller's module check prints its warning counts after its refusal; a failed check +// is said by the refusal, never by the count printed last. +func TestAFailedModuleCheckIsSaidByItsRefusalNotItsWarningCount(t *testing.T) { + log := "mesh-controller: 3 problem(s) in 139 manifest(s)\n" + + "mesh-files claims node-files and says it serves upload, download, which that seat's protocol does not promise\n" + + "minio claims the seat \"object-store\", which no module declares and the mesh does not define\n" + + "only-office claims the seat \"document-converter\", which no module declares and the mesh does not define\n" + + "adwaita: ok, 4 tool(s)\nlong-running resources without health: 59\n" + + "files asking for a setting without saying whether it is trusted: 36\n" + // 1310/AC1 + want := "mesh-controller: 3 problem(s) in 139 manifest(s): mesh-files claims node-files and says it serves upload, " + + "download, which that seat's protocol does not promise" + if got := whatFailed(log); got != want { + t.Errorf("1310/AC1: the captured log is said as %q, not %q", got, want) + } + // 1310/AC2 + if got, want := whatFailed("ok \tx/a\t1s\n--- FAIL: TestX (0.01s)\n x_test.go:12: got 1\nFAIL\nlong-running resources without health: 59\n"), + "--- FAIL: TestX (0.01s)"; got != want { + t.Errorf("1310/AC2: a failing test is said as %q, not %q", got, want) + } + // 1310/AC3 + if got, want := whatFailed("starting\nsomething odd happened\n"), "something odd happened"; got != want { + t.Errorf("1310/AC3: a log with no failure line is said as %q, not %q", got, want) + } +} From 2377d75ed89840b51f7cc21f70267858b7ce5150 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 18:57:59 +0200 Subject: [PATCH 2/8] Say a building plan's refusal in its line, not 'building' with LATE, once its tier is built (issue 920) --- cmd/mesh-controller/plan_clock_test.go | 31 ++++++++++++++++++++++++++ cmd/mesh-controller/release_plan.go | 21 +++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/cmd/mesh-controller/plan_clock_test.go b/cmd/mesh-controller/plan_clock_test.go index 49631e53..ef4b8156 100644 --- a/cmd/mesh-controller/plan_clock_test.go +++ b/cmd/mesh-controller/plan_clock_test.go @@ -78,3 +78,34 @@ func TestAPlansLineCountsFromItsTierNotFromItsLastSave(t *testing.T) { // novox/hq issue 296: a plan whose tier stands still is not written again by every advance — its revision, // its last save and its `plan-moved` stay where its last change left them. + +// novox/hq issue 920: a building plan whose current tier is all built waits on its note (a send refused at the +// gate), and its line, `status` and the stalled condition say that, not "building for ... LATE". +func TestAPlanLineSaysTheRefusalOnceItsTierIsBuilt(t *testing.T) { + now := time.Date(2026, 10, 21, 19, 0, 0, 0, time.UTC) + built := now.Add(-time.Hour) + refusal := "tier 1: a build waits there for its gate: novox (mesh-delivery-indexer d68d7ef3→a5aa69a4) — tried again" + mk := func(second string) inventory.Plan { + return inventory.Plan{ID: "plan-r", Repository: "novox/mesh-sdk", Commit: "b2f5ebb2", State: inventory.PlanBuilding, + Tier: 1, Created: now.Add(-2 * time.Hour), Updated: now, TierEntered: now.Add(-time.Hour), Note: refusal, + Tiers: [][]string{{"a"}, {"b", "c"}}, + Modules: map[string]*inventory.PlanModule{"a": {State: "built", BuiltAt: &built}, + "b": {State: "built", BuiltAt: &built}, "c": {State: second}}} + } + + // 920/AC1 + p := mk("built") + line := planLine(p, now) + if !strings.Contains(line, "a build waits there for its gate: novox") || strings.Contains(line, "building for") { + t.Errorf("920/AC1: a plan with its tier built and a refusal reads %q", line) + } + st := planStatuses([]inventory.Plan{p}, now, pauseView{}, nil) + if len(st) != 1 || !st[0].Late { + t.Errorf("920/AC1: status: %+v", st) + } + + // 920/AC2 + if line := planLine(mk("asked"), now); !strings.Contains(line, "building for 1h0m0s") { + t.Errorf("920/AC2: a plan with a tier module not built reads %q", line) + } +} diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 7331626c..5ca93c4e 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -863,6 +863,20 @@ func deferred(p inventory.Plan) bool { p.Delivery.Awaits != "" && p.Delivery.Go != nil && strings.HasPrefix(p.Note, deferredNote) } +// currentTierBuilt says every module of a plan's current tier is built (or deleted), so no build is what it waits on. +func currentTierBuilt(p inventory.Plan) bool { + if p.Tier < 0 || p.Tier >= len(p.Tiers) || len(p.Tiers[p.Tier]) == 0 { + return false + } + for _, m := range p.Tiers[p.Tier] { + s := p.Modules[m] + if s == nil || (s.State != "built" && s.State != planDeleted) { + return false + } + } + return true +} + // askedAny says a plan asked any module. func askedAny(p inventory.Plan) bool { for _, s := range p.Modules { @@ -2025,6 +2039,13 @@ func planLineSaid(p inventory.Plan, now time.Time, pause pauseView, bound time.D if p.State == inventory.PlanRolling { what = p.Note } + if p.State == inventory.PlanBuilding && p.Note != "" && currentTierBuilt(p) { + // Every build of the tier is in, so the plan waits on something else, and its note says what: a send + // refused ("a build waits there for its gate: (...)") is kept there. "building" with LATE + // hid that reason from `plans`, `status` and the stalled condition (novox/hq issue 920). + what = p.Note + return fmt.Sprintf("%s · %s, held: %s, for %s%s", name, where, what, since, late) + } return fmt.Sprintf("%s · %s, %s for %s%s", name, where, what, since, late) } From 612f493c4926cffc50f171eebf716f61abc7ae36 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 19:01:42 +0200 Subject: [PATCH 3/8] Say only a refusal as a plan's hold, not a tier's summary left in its note (issue 920, review round 1) --- cmd/mesh-controller/plan_clock_test.go | 16 ++++++++++++++++ cmd/mesh-controller/release_plan.go | 21 +++++++++++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/cmd/mesh-controller/plan_clock_test.go b/cmd/mesh-controller/plan_clock_test.go index ef4b8156..36073456 100644 --- a/cmd/mesh-controller/plan_clock_test.go +++ b/cmd/mesh-controller/plan_clock_test.go @@ -109,3 +109,19 @@ func TestAPlanLineSaysTheRefusalOnceItsTierIsBuilt(t *testing.T) { t.Errorf("920/AC2: a plan with a tier module not built reads %q", line) } } + +// 920/AC3: a tier move writes the previous tier's summary into the note; with the new tier all built, the line still +// says "building for", for that note is no refusal. +func TestAPlanLineDoesNotHoldOnATiersSummary(t *testing.T) { + now := time.Date(2026, 10, 21, 19, 0, 0, 0, time.UTC) + built := now.Add(-time.Hour) + for _, note := range []string{"failed or refused its send (apply-failing says it): novox", "tier 0 built; sent mesh-sdk to novox first, in one send"} { + p := inventory.Plan{ID: "plan-s", Repository: "novox/mesh-sdk", Commit: "b2f5ebb2", State: inventory.PlanBuilding, + Tier: 1, Created: now.Add(-2 * time.Hour), Updated: now, TierEntered: now.Add(-time.Hour), Note: note, + Tiers: [][]string{{"a"}, {"b"}}, + Modules: map[string]*inventory.PlanModule{"a": {State: "built", BuiltAt: &built}, "b": {State: "built", BuiltAt: &built}}} + if line := planLine(p, now); !strings.Contains(line, "building for 1h0m0s") || strings.Contains(line, "held") { + t.Errorf("920/AC3: a plan with the note %q reads %q", note, line) + } + } +} diff --git a/cmd/mesh-controller/release_plan.go b/cmd/mesh-controller/release_plan.go index 5ca93c4e..430cb335 100644 --- a/cmd/mesh-controller/release_plan.go +++ b/cmd/mesh-controller/release_plan.go @@ -851,6 +851,23 @@ func startedBeside(ctx context.Context, inv *inventory.Inventory, p inventory.Pl return behind, why, nil } +// refusalNotePrefix and refusalNoteSuffix frame the note of a step the walk was refused: "tier N: — tried +// again". Only a note so framed is a hold (novox/hq issue 920); a tier's summary left in the note is not. +const ( + refusalNotePrefix = "tier " + refusalNoteSuffix = " — tried again" +) + +// refusalNote is the note of a plan whose step at tier was refused with err. +func refusalNote(tier int, err error) string { + return refusalNotePrefix + fmt.Sprint(tier) + ": " + err.Error() + refusalNoteSuffix +} + +// refused says a plan's note is a refusal the walk wrote. +func refused(p inventory.Plan) bool { + return strings.HasPrefix(p.Note, refusalNotePrefix) && strings.HasSuffix(p.Note, refusalNoteSuffix) +} + // deferredNote begins the note of a walk deferred behind another. const deferredNote = "let go; starts once " @@ -941,7 +958,7 @@ func advanceHeld(ctx context.Context, open *stores) (failed bool) { // Kept in the plan, so `plans` says why it has not moved rather than the log alone; // the state is left as it was and the step is tried again by the next look. Said and // kept when it is new: the same refusal on every tick is one fact, not one per tick. - p.Note = "tier " + fmt.Sprint(p.Tier) + ": " + err.Error() + " — tried again" + p.Note = refusalNote(p.Tier, err) // A refusal for the bus's planned step is a person's to end: S17 says it from its first moment. if refusedForTheBus(err) { busRefusedFirst.mark(p.ID, time.Now()) @@ -2039,7 +2056,7 @@ func planLineSaid(p inventory.Plan, now time.Time, pause pauseView, bound time.D if p.State == inventory.PlanRolling { what = p.Note } - if p.State == inventory.PlanBuilding && p.Note != "" && currentTierBuilt(p) { + if p.State == inventory.PlanBuilding && refused(p) && currentTierBuilt(p) { // Every build of the tier is in, so the plan waits on something else, and its note says what: a send // refused ("a build waits there for its gate: (...)") is kept there. "building" with LATE // hid that reason from `plans`, `status` and the stalled condition (novox/hq issue 920). From deac3e232ccb38d8a1283765706e246bbd17a707 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 19:21:11 +0200 Subject: [PATCH 4/8] Refuse a process account's stand-in, naming issue (issue 1226) --- cmd/mesh-controller/compose_perform.go | 10 ++ cmd/mesh-controller/process_accounts.go | 120 +++++++++++++++++++ cmd/mesh-controller/process_accounts_test.go | 42 +++++++ 3 files changed, 172 insertions(+) create mode 100644 cmd/mesh-controller/process_accounts.go create mode 100644 cmd/mesh-controller/process_accounts_test.go diff --git a/cmd/mesh-controller/compose_perform.go b/cmd/mesh-controller/compose_perform.go index c71c4bfe..ed29ac69 100644 --- a/cmd/mesh-controller/compose_perform.go +++ b/cmd/mesh-controller/compose_perform.go @@ -233,6 +233,16 @@ func prepareSend(ctx context.Context, open *stores, node string) ([]string, erro if !slices.Contains(in.BusMinted, "provisioner."+node) { continue } + default: + // A bus process's own account is minted by `issue`, never made as a random value (issue 1226). + if err := processAccountRefusal(m, node, name, in.BusMinted); err != nil { + return nil, err + } + if strings.HasPrefix(name, "broker-") && len(m.ProcessBuses()) > 0 { + if _, isProcess := m.ProcessBuses()[strings.TrimPrefix(name, "broker-")]; isProcess { + continue // issued, and not held under this key: issued again, never made + } + } } if _, err := inv.SecretForModule(ctx, node, m.Module, name); err == nil { did = true diff --git a/cmd/mesh-controller/process_accounts.go b/cmd/mesh-controller/process_accounts.go new file mode 100644 index 00000000..4dee11d3 --- /dev/null +++ b/cmd/mesh-controller/process_accounts.go @@ -0,0 +1,120 @@ +package main + +import ( + "context" + "fmt" + "strings" + + "git.novox.be/novox/mesh-sdk/go/catalogue" + + "github.com/novox/mesh-controller/internal/broker" + "github.com/novox/mesh-controller/internal/inventory" +) + +// processUser is the bus user of one process of a module on a node (novox/hq ADR 0436 §1). +func processUser(node, module, process string) string { + return broker.Principal{Kind: broker.KindProcess, Node: node, Module: module, Process: process}.Username() +} + +// processAccountRefusal is the refusal for the own secret broker- of a process that declares bus when +// nobody issued its account (novox/hq ADR 0436 §2, issue 1226): the account is never invented as a random value +// the process cannot read, it is minted by `mesh-controller.issue`. Nil for any other secret, and for an account +// that is issued. +func processAccountRefusal(m catalogue.Manifest, node, secret string, minted []string) error { + id, isProcess := strings.CutPrefix(secret, "broker-") + if !isProcess { + return nil + } + if _, declared := m.ProcessBuses()[id]; !declared { + return nil + } + user := processUser(node, m.Module, id) + for _, u := range minted { + if u == user { + return nil + } + } + return fmt.Errorf("%s on %s has no account for its process %s: nothing was issued for %s, and a push would "+ + "seal a random value its process cannot read (novox/hq ADR 0436 §2, issue 1226). "+ + "`mesh-controller.issue %s %s` (module issue %s --node %s), then push again", + m.Module, node, id, user, node, m.Module, m.Module, node) +} + +// processAccountsMissing is each process of m that declares bus and whose account is not minted yet on node. +func processAccountsMissing(m catalogue.Manifest, node string, minted []string) []broker.ProcessPrincipal { + have := map[string]bool{} + for _, u := range minted { + have[u] = true + } + var out []broker.ProcessPrincipal + for _, pr := range inventory.ProcessesOf(m) { + if !have[processUser(node, m.Module, pr.Process)] { + out = append(out, pr) + } + } + return out +} + +// issueMissingProcessAccounts issues the accounts of m's bus processes on node that are not minted, and no +// other: an account already issued is rotated on purpose, never by a later delivery or assignment (issue 1226). +// It says what it issued, one line, and nothing when nothing was missing. +func issueMissingProcessAccounts(ctx context.Context, inv *inventory.Inventory, m catalogue.Manifest, node string) (string, error) { + if len(m.ProcessBuses()) == 0 || mayIssue(m) != nil { + return "", nil + } + minted, err := inv.BusMinted(ctx) + if err != nil { + return "", err + } + missing := processAccountsMissing(m, node, minted) + if len(missing) == 0 { + return "", nil + } + known, err := broker.FromEnvironment() + if err != nil { + return "", fmt.Errorf("cannot deliver a credential without knowing where the bus is: %w", err) + } + reachable, err := brokerReachableAt(ctx, inv, known, node) + if err != nil { + return "", err + } + busAddress, err := broker.BusAddress() + if err != nil { + busAddress = "" + } + var ids []string + for _, pr := range missing { + if err := issueProcess(ctx, inv, m, pr, node, busAddress, known, reachable); err != nil { + return "", err + } + ids = append(ids, pr.Process) + } + return fmt.Sprintf("its process account(s) %s issued and sealed to %s, and arrive with the push", + strings.Join(ids, ", "), node), nil +} + +// issueGainedProcessAccounts runs after a module's new definition is registered: on each machine assigned it, the +// accounts of its bus processes not yet minted are issued, so a process a delivery adds is not pushed with a +// random stand-in (issue 1226). Never fatal: the build worked and the module is registered, so a failure is said, +// and the push refuses the module until `mesh-controller.issue` is run. +func issueGainedProcessAccounts(ctx context.Context, inv *inventory.Inventory, m catalogue.Manifest) { + if len(m.ProcessBuses()) == 0 { + return + } + nodes, err := inv.Running(ctx, m.Module) + if err != nil { + fmt.Printf("%s: its machines are not read to issue its process accounts: %v\n", m.Module, err) + return + } + for _, node := range nodes { + line, err := issueMissingProcessAccounts(ctx, inv, m, node) + if err != nil { + fmt.Printf("%s on %s: its process accounts are not issued (%v): `mesh-controller.issue %s %s` first\n", + m.Module, node, err, node, m.Module) + continue + } + if line != "" { + fmt.Printf("%s on %s: %s\n", m.Module, node, line) + } + } +} diff --git a/cmd/mesh-controller/process_accounts_test.go b/cmd/mesh-controller/process_accounts_test.go new file mode 100644 index 00000000..84c7fea8 --- /dev/null +++ b/cmd/mesh-controller/process_accounts_test.go @@ -0,0 +1,42 @@ +package main + +import ( + "strings" + "testing" +) + +// 1226/AC1: given a module whose process declares bus and has no issued account, when a machine's declaration +// is composed, the push refuses that module, naming issue. (The composition's wiring is proven against the store +// in TestAGainedProcessIsIssuedWhenTheDeliveryRecordsIt.) +func TestAProcessAccountNobodyIssuedIsRefusedNamingIssue(t *testing.T) { + m := mailSenderWithProcess() + err := processAccountRefusal(m, "laptop", "broker-send", nil) + if err == nil { + t.Fatal("1226/AC1: a process account nobody issued was not refused, so a random value would be made") + } + if !strings.Contains(err.Error(), "mesh-controller.issue laptop mesh-mail") { + t.Fatalf("1226/AC1: the refusal does not name issue: %v", err) + } + // Other names are not this rule's: the module's own broker and a secret of no process. + if processAccountRefusal(m, "laptop", "broker", nil) != nil || processAccountRefusal(m, "laptop", "broker-other", nil) != nil { + t.Fatal("1226/AC1: a name that is no declared process's account was refused") + } + // Issued, it is not refused. + if err := processAccountRefusal(m, "laptop", "broker-send", []string{"laptop.mesh-mail.send"}); err != nil { + t.Fatalf("1226/AC1: an issued account was refused: %v", err) + } +} + +// 1226/AC3: an account already issued is not replaced by a later compose, nor issued again by a delivery. +func TestAnIssuedProcessAccountIsNotIssuedAgain(t *testing.T) { + m := mailSenderWithProcess() + if got := processAccountsMissing(m, "laptop", nil); len(got) != 1 || got[0].Process != "send" { + t.Fatalf("1226/AC3: an account not issued is not missing: %v", got) + } + if got := processAccountsMissing(m, "laptop", []string{"laptop.mesh-mail.send"}); len(got) != 0 { + t.Fatalf("1226/AC3: an issued account would be issued again: %v", got) + } + if got := processAccountsMissing(m, "ace", []string{"laptop.mesh-mail.send"}); len(got) != 1 { + t.Fatalf("1226/AC3: another machine's account stands in for this one's: %v", got) + } +} From 6bcfbeb5b2386c6c5d6de5b8a50b6eb6f2e071e4 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 19:21:11 +0200 Subject: [PATCH 5/8] Issue a bus process's account when its module is assigned or gains it (issue 1226) --- cmd/mesh-controller/acts.go | 23 ++++++- cmd/mesh-controller/build.go | 3 + .../process_accounts_integration_test.go | 64 +++++++++++++++++++ 3 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 cmd/mesh-controller/process_accounts_integration_test.go diff --git a/cmd/mesh-controller/acts.go b/cmd/mesh-controller/acts.go index 3989f0f8..9a8cd694 100644 --- a/cmd/mesh-controller/acts.go +++ b/cmd/mesh-controller/acts.go @@ -408,12 +408,21 @@ func issueOnAssign(ctx context.Context, open *stores, node, module string) strin return "" } user := broker.Principal{Kind: broker.KindModule, Node: node, Module: module}.Username() - if _, reads := m.OwnSecrets["broker"]; !reads { + _, reads := m.OwnSecrets["broker"] + if !reads && !runsTheProvisionerClient(m) { + // A module whose bus accounts are its processes' alone has none of its own: only the processes that + // are not issued yet are issued (issue 1226). + return processAccountsOnAssign(ctx, inv, m, node) + } + if !reads { // A module running the provisioner client alone is minted the client's account (issue 917). user = broker.Principal{Kind: broker.KindProvisioner, Node: node, Module: module}.Username() } - if _, minted, err := inv.BusUserHash(ctx, user); err != nil || minted { + if _, minted, err := inv.BusUserHash(ctx, user); err != nil { return "" + } else if minted { + // Its own account stands and is kept; a process it gained is not issued yet (issue 1226). + return processAccountsOnAssign(ctx, inv, m, node) } busAddress, err := broker.BusAddress() if err == nil { @@ -426,6 +435,16 @@ func issueOnAssign(ctx context.Context, open *stores, node, module string) strin return fmt.Sprintf("its bus credential is issued and sealed to %s, and arrives with the push", node) } +// processAccountsOnAssign issues the process accounts of an assigned module not issued yet, and says what it did. +func processAccountsOnAssign(ctx context.Context, inv *inventory.Inventory, m catalogue.Manifest, node string) string { + line, err := issueMissingProcessAccounts(ctx, inv, m, node) + if err != nil { + return fmt.Sprintf("its process accounts are not issued (%v): `module issue %s --node %s` first — "+ + "`push %s` refuses to send %s until it is", err, m.Module, node, node, m.Module) + } + return line +} + // refusedMovingTheController refuses assigning or unassigning the controller's module through a verb (review of // novox/hq ADR 0272): the node it is assigned to is the control-node, and the control-node's operator account is // the controller's terminal, so whoever moves the module chooses the terminal. At the terminal alone, as every diff --git a/cmd/mesh-controller/build.go b/cmd/mesh-controller/build.go index accc7e29..b7077f1b 100644 --- a/cmd/mesh-controller/build.go +++ b/cmd/mesh-controller/build.go @@ -768,6 +768,9 @@ func takeIn(ctx context.Context, inv *inventory.Inventory, result link.BuildResu } return manifest, kept, err } + // A bus process the new definition gains gets its account now, on each machine assigned the module, so the + // next push composes a real account (issue 1226). + issueGainedProcessAccounts(ctx, inv, manifest) // Which repository it is registered from, by the forge's own id (novox/hq ADR 0266). if err := inv.SetSourceIdentity(ctx, manifest.Module, repoID); err != nil { return manifest, kept, err diff --git a/cmd/mesh-controller/process_accounts_integration_test.go b/cmd/mesh-controller/process_accounts_integration_test.go new file mode 100644 index 00000000..983fbf78 --- /dev/null +++ b/cmd/mesh-controller/process_accounts_integration_test.go @@ -0,0 +1,64 @@ +//go:build integration + +package main + +import ( + "path/filepath" + "strings" + "testing" + + "git.novox.be/novox/mesh-sdk/go/catalogue" + + "github.com/novox/mesh-controller/internal/broker" +) + +// 1226/AC2: given an assigned module whose new definition adds a bus process, when the delivery records it, +// that process's account is issued and the next compose carries a real account; and (AC1) before it, the +// composition refuses the module naming issue; (AC3) a later record does not replace the account. +func TestAGainedProcessIsIssuedWhenTheDeliveryRecordsIt(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + inv := open.inventory + + dir := t.TempDir() + if err := busCertificate([]string{"--into", dir}); err != nil { + t.Fatal(err) + } + t.Setenv(broker.AddressVar, "127.0.0.1:4222") + t.Setenv(broker.CertificateVar, filepath.Join(dir, "tls.crt")) + + // Assigned without the process, then a definition that gains it. + plain, err := catalogue.ParseManifest([]byte(`{"module":"mesh-mail","version":"1", + "own-secrets":{"broker-send":{"path":"/var/lib/mesh-mail/broker-send","owner":"mesh-mail-send"}}, + "resources":[{"id":"account","type":"user","name":"mesh-mail-send"}]}`)) + if err != nil { + t.Fatal(err) + } + register(t, open, plain) + if _, err := assign(ctx, open, "laptop", "mesh-mail"); err != nil { + t.Fatal(err) + } + gained := mailSenderWithProcess() + register(t, open, gained) + + // 1226/AC1 on the store: nothing issued, so the compose refuses, naming issue. + if _, err := prepareSend(ctx, open, "laptop"); err == nil || !strings.Contains(err.Error(), "mesh-controller.issue laptop mesh-mail") { + t.Fatalf("1226/AC1: the composition did not refuse the unissued account naming issue: %v", err) + } + + issueGainedProcessAccounts(ctx, inv, gained) + hash, held, err := inv.BusUserHash(ctx, "laptop.mesh-mail.send") + if err != nil || !held { + t.Fatalf("1226/AC2: the gained process's account was not issued: held %v, %v", held, err) + } + if _, err := prepareSend(ctx, open, "laptop"); err != nil { + t.Fatalf("1226/AC2: the next compose does not carry the account: %v", err) + } + + // 1226/AC3: recording the definition again, and composing, replaces nothing. + issueGainedProcessAccounts(ctx, inv, gained) + again, _, err := inv.BusUserHash(ctx, "laptop.mesh-mail.send") + if err != nil || again != hash { + t.Fatalf("1226/AC3: the issued account was replaced (%v)", err) + } +} From faf8beee84930c09d257aa7d06524c66a8df37f4 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 19:25:44 +0200 Subject: [PATCH 6/8] Warn instead of holding the machine for an unissued process account, and drive the delivery path in the test (issue 1226, review round 1) --- cmd/mesh-controller/compose_perform.go | 12 +-- cmd/mesh-controller/process_accounts.go | 34 ++++--- .../process_accounts_integration_test.go | 94 ++++++++++++++----- cmd/mesh-controller/process_accounts_test.go | 36 +++---- 4 files changed, 109 insertions(+), 67 deletions(-) diff --git a/cmd/mesh-controller/compose_perform.go b/cmd/mesh-controller/compose_perform.go index ed29ac69..6e05f2bd 100644 --- a/cmd/mesh-controller/compose_perform.go +++ b/cmd/mesh-controller/compose_perform.go @@ -234,14 +234,10 @@ func prepareSend(ctx context.Context, open *stores, node string) ([]string, erro continue } default: - // A bus process's own account is minted by `issue`, never made as a random value (issue 1226). - if err := processAccountRefusal(m, node, name, in.BusMinted); err != nil { - return nil, err - } - if strings.HasPrefix(name, "broker-") && len(m.ProcessBuses()) > 0 { - if _, isProcess := m.ProcessBuses()[strings.TrimPrefix(name, "broker-")]; isProcess { - continue // issued, and not held under this key: issued again, never made - } + // A bus process's account nobody issued still gets the stand-in, until the composition leaves + // the module off (issue 1753): the warning names the verb (issue 1226). + if line := processAccountWarning(m, node, name, in.BusMinted); line != "" { + sayWarning(ctx, line) } } if _, err := inv.SecretForModule(ctx, node, m.Module, name); err == nil { diff --git a/cmd/mesh-controller/process_accounts.go b/cmd/mesh-controller/process_accounts.go index 4dee11d3..43bbc84d 100644 --- a/cmd/mesh-controller/process_accounts.go +++ b/cmd/mesh-controller/process_accounts.go @@ -3,6 +3,8 @@ package main import ( "context" "fmt" + "io" + "os" "strings" "git.novox.be/novox/mesh-sdk/go/catalogue" @@ -16,28 +18,36 @@ func processUser(node, module, process string) string { return broker.Principal{Kind: broker.KindProcess, Node: node, Module: module, Process: process}.Username() } -// processAccountRefusal is the refusal for the own secret broker- of a process that declares bus when -// nobody issued its account (novox/hq ADR 0436 §2, issue 1226): the account is never invented as a random value -// the process cannot read, it is minted by `mesh-controller.issue`. Nil for any other secret, and for an account -// that is issued. -func processAccountRefusal(m catalogue.Manifest, node, secret string, minted []string) error { +// processAccountWarning is the warning line for the own secret broker- of a process that declares bus +// when nobody issued its account (novox/hq ADR 0436 §2, issue 1226): "" for any other secret and for an issued +// account. The send goes on with the stand-in until the composition leaves the module off (issue 1753), so the +// line is the only loud thing about it. +func processAccountWarning(m catalogue.Manifest, node, secret string, minted []string) string { id, isProcess := strings.CutPrefix(secret, "broker-") if !isProcess { - return nil + return "" } if _, declared := m.ProcessBuses()[id]; !declared { - return nil + return "" } user := processUser(node, m.Module, id) for _, u := range minted { if u == user { - return nil + return "" } } - return fmt.Errorf("%s on %s has no account for its process %s: nothing was issued for %s, and a push would "+ - "seal a random value its process cannot read (novox/hq ADR 0436 §2, issue 1226). "+ - "`mesh-controller.issue %s %s` (module issue %s --node %s), then push again", - m.Module, node, id, user, node, m.Module, m.Module, node) + return fmt.Sprintf("%s/%s process %s: its bus account is not issued — mesh-controller.issue %s %s", + node, m.Module, id, node, m.Module) +} + +// sayWarning writes a line where the compose's other warnings go: the act's answer when it carries a writer, +// else the controller's journal. +func sayWarning(ctx context.Context, line string) { + var w io.Writer = os.Stderr + if lw, ok := ctx.Value(leftOutKey{}).(io.Writer); ok { + w = lw + } + fmt.Fprintln(w, line) } // processAccountsMissing is each process of m that declares bus and whose account is not minted yet on node. diff --git a/cmd/mesh-controller/process_accounts_integration_test.go b/cmd/mesh-controller/process_accounts_integration_test.go index 983fbf78..8c735ed6 100644 --- a/cmd/mesh-controller/process_accounts_integration_test.go +++ b/cmd/mesh-controller/process_accounts_integration_test.go @@ -3,6 +3,8 @@ package main import ( + "bytes" + "encoding/json" "path/filepath" "strings" "testing" @@ -10,11 +12,24 @@ import ( "git.novox.be/novox/mesh-sdk/go/catalogue" "github.com/novox/mesh-controller/internal/broker" + "github.com/novox/mesh-controller/internal/link" + "github.com/novox/mesh-controller/internal/testbus" ) -// 1226/AC2: given an assigned module whose new definition adds a bus process, when the delivery records it, -// that process's account is issued and the next compose carries a real account; and (AC1) before it, the -// composition refuses the module naming issue; (AC3) a later record does not replace the account. +func gainedAndPlain(t *testing.T) (plain, gained catalogue.Manifest) { + t.Helper() + plain, err := catalogue.ParseManifest([]byte(`{"module":"mesh-mail","version":"1", + "own-secrets":{"broker-send":{"path":"/var/lib/mesh-mail/broker-send","owner":"mesh-mail-send"}}, + "resources":[{"id":"account","type":"user","name":"mesh-mail-send"}]}`)) + if err != nil { + t.Fatal(err) + } + return plain, mailSenderWithProcess() +} + +// 1226/AC2: given an assigned module whose new definition adds a bus process, when the delivery records it +// (takeIn, the function that records a build result), the process's account is minted; and 1226/AC3: the same +// build heard again replaces nothing. func TestAGainedProcessIsIssuedWhenTheDeliveryRecordsIt(t *testing.T) { open := aMesh(t) ctx := t.Context() @@ -24,41 +39,68 @@ func TestAGainedProcessIsIssuedWhenTheDeliveryRecordsIt(t *testing.T) { if err := busCertificate([]string{"--into", dir}); err != nil { t.Fatal(err) } - t.Setenv(broker.AddressVar, "127.0.0.1:4222") + t.Setenv(broker.AddressVar, strings.TrimPrefix(testbus.URL(t), "nats://")) t.Setenv(broker.CertificateVar, filepath.Join(dir, "tls.crt")) - // Assigned without the process, then a definition that gains it. - plain, err := catalogue.ParseManifest([]byte(`{"module":"mesh-mail","version":"1", - "own-secrets":{"broker-send":{"path":"/var/lib/mesh-mail/broker-send","owner":"mesh-mail-send"}}, - "resources":[{"id":"account","type":"user","name":"mesh-mail-send"}]}`)) - if err != nil { - t.Fatal(err) - } + plain, gained := gainedAndPlain(t) register(t, open, plain) if _, err := assign(ctx, open, "laptop", "mesh-mail"); err != nil { t.Fatal(err) } - gained := mailSenderWithProcess() - register(t, open, gained) - - // 1226/AC1 on the store: nothing issued, so the compose refuses, naming issue. - if _, err := prepareSend(ctx, open, "laptop"); err == nil || !strings.Contains(err.Error(), "mesh-controller.issue laptop mesh-mail") { - t.Fatalf("1226/AC1: the composition did not refuse the unissued account naming issue: %v", err) + if _, minted, _ := inv.BusUserHash(ctx, "laptop.mesh-mail.send"); minted { + t.Fatal("the account was minted before the module gained the process") } - issueGainedProcessAccounts(ctx, inv, gained) + manifest, _ := json.Marshal(gained) + build := func(id, commit string) { + t.Helper() + if _, _, err := takeIn(ctx, inv, asTheOperator(t, inv, link.BuildResult{ + ID: id, Repository: "http://forge.internal:20000/novox/mesh-mail.git", Path: "modules/mesh-mail", + Ref: "main", On: "anchor", Commit: commit, Manifest: manifest, + Source: &link.SourceOnSeat{Seat: "git", Repository: "novox/mesh-mail"}, + })); err != nil { + t.Fatal(err) + } + } + build("b-1", "aaaaaaa1111") hash, held, err := inv.BusUserHash(ctx, "laptop.mesh-mail.send") if err != nil || !held { - t.Fatalf("1226/AC2: the gained process's account was not issued: held %v, %v", held, err) + t.Fatalf("1226/AC2: recording the gained definition did not mint the process's account: held %v, %v", held, err) } - if _, err := prepareSend(ctx, open, "laptop"); err != nil { - t.Fatalf("1226/AC2: the next compose does not carry the account: %v", err) - } - - // 1226/AC3: recording the definition again, and composing, replaces nothing. - issueGainedProcessAccounts(ctx, inv, gained) + build("b-2", "bbbbbbb2222") again, _, err := inv.BusUserHash(ctx, "laptop.mesh-mail.send") if err != nil || again != hash { - t.Fatalf("1226/AC3: the issued account was replaced (%v)", err) + t.Fatalf("1226/AC3: recording again replaced the issued account (%v)", err) + } +} + +// 1226/AC1 and AC4 against the store: a process account nobody issued is warned of in the push's answer, naming +// issue, and the send is not refused, so the machine's other modules are composed. +func TestAnUnissuedProcessAccountIsWarnedOfAndTheRestIsComposed(t *testing.T) { + open := aMesh(t) + ctx := t.Context() + _, gained := gainedAndPlain(t) + register(t, open, gained) + other := catalogue.Manifest{Module: "other", Version: "1", + OwnSecrets: catalogue.OwnSecrets{"token": {Path: "/var/lib/other/token"}}} + register(t, open, other) + for _, m := range []string{"mesh-mail", "other"} { + if _, err := assign(ctx, open, "laptop", m); err != nil { + t.Fatal(err) + } + } + var said bytes.Buffer + if _, err := prepareSend(leftOutSaidBy(ctx, &said), open, "laptop"); err != nil { + t.Fatalf("1226/AC4: an unissued process account held the machine: %v", err) + } + want := "laptop/mesh-mail process send: its bus account is not issued — mesh-controller.issue laptop mesh-mail" + if !strings.Contains(said.String(), want) { + t.Fatalf("1226/AC1: the push says %q, want %q", said.String(), want) + } + if strings.Contains(said.String(), "other") { + t.Fatalf("1226/AC4: the other module is warned of: %q", said.String()) + } + if _, held, err := open.inventory.ModuleSecretIfIssued(ctx, "laptop", "other", "token"); err != nil || !held { + t.Fatalf("1226/AC4: the other module's secret was not composed: held %v, %v", held, err) } } diff --git a/cmd/mesh-controller/process_accounts_test.go b/cmd/mesh-controller/process_accounts_test.go index 84c7fea8..d9e3f2af 100644 --- a/cmd/mesh-controller/process_accounts_test.go +++ b/cmd/mesh-controller/process_accounts_test.go @@ -1,33 +1,27 @@ package main -import ( - "strings" - "testing" -) +import "testing" -// 1226/AC1: given a module whose process declares bus and has no issued account, when a machine's declaration -// is composed, the push refuses that module, naming issue. (The composition's wiring is proven against the store -// in TestAGainedProcessIsIssuedWhenTheDeliveryRecordsIt.) -func TestAProcessAccountNobodyIssuedIsRefusedNamingIssue(t *testing.T) { +// 1226/AC1: given a module whose process declares bus and has no issued account, when a machine is composed, the +// push's warning names issue, and the machine's other modules are composed (AC4: nothing is refused). +func TestAProcessAccountNobodyIssuedIsWarnedOfNamingIssue(t *testing.T) { m := mailSenderWithProcess() - err := processAccountRefusal(m, "laptop", "broker-send", nil) - if err == nil { - t.Fatal("1226/AC1: a process account nobody issued was not refused, so a random value would be made") + got := processAccountWarning(m, "laptop", "broker-send", nil) + want := "laptop/mesh-mail process send: its bus account is not issued — mesh-controller.issue laptop mesh-mail" + if got != want { + t.Fatalf("1226/AC1: the warning is %q, want %q", got, want) } - if !strings.Contains(err.Error(), "mesh-controller.issue laptop mesh-mail") { - t.Fatalf("1226/AC1: the refusal does not name issue: %v", err) + // Other names are not this rule's. + if processAccountWarning(m, "laptop", "broker", nil) != "" || processAccountWarning(m, "laptop", "broker-other", nil) != "" || + processAccountWarning(m, "laptop", "token", nil) != "" { + t.Fatal("1226/AC1: a name that is no declared process's account was warned of") } - // Other names are not this rule's: the module's own broker and a secret of no process. - if processAccountRefusal(m, "laptop", "broker", nil) != nil || processAccountRefusal(m, "laptop", "broker-other", nil) != nil { - t.Fatal("1226/AC1: a name that is no declared process's account was refused") - } - // Issued, it is not refused. - if err := processAccountRefusal(m, "laptop", "broker-send", []string{"laptop.mesh-mail.send"}); err != nil { - t.Fatalf("1226/AC1: an issued account was refused: %v", err) + if got := processAccountWarning(m, "laptop", "broker-send", []string{"laptop.mesh-mail.send"}); got != "" { + t.Fatalf("1226/AC3: an issued account was warned of: %q", got) } } -// 1226/AC3: an account already issued is not replaced by a later compose, nor issued again by a delivery. +// 1226/AC3: an account already issued is not issued again by a later delivery or assignment. func TestAnIssuedProcessAccountIsNotIssuedAgain(t *testing.T) { m := mailSenderWithProcess() if got := processAccountsMissing(m, "laptop", nil); len(got) != 1 || got[0].Process != "send" { From b08c6372e135d172640848071774fbf9463b4e2f Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 19:26:57 +0200 Subject: [PATCH 7/8] Assert the next compose carries the issued account (issue 1226, review round 2) --- .../process_accounts_integration_test.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/cmd/mesh-controller/process_accounts_integration_test.go b/cmd/mesh-controller/process_accounts_integration_test.go index 8c735ed6..1c2e3db5 100644 --- a/cmd/mesh-controller/process_accounts_integration_test.go +++ b/cmd/mesh-controller/process_accounts_integration_test.go @@ -67,6 +67,17 @@ func TestAGainedProcessIsIssuedWhenTheDeliveryRecordsIt(t *testing.T) { if err != nil || !held { t.Fatalf("1226/AC2: recording the gained definition did not mint the process's account: held %v, %v", held, err) } + // And the next compose carries the account, with no warning (1226/AC2). + var said bytes.Buffer + if _, err := prepareSend(leftOutSaidBy(ctx, &said), open, "laptop"); err != nil { + t.Fatalf("1226/AC2: the next compose failed: %v", err) + } + if strings.Contains(said.String(), "bus account is not issued") { + t.Fatalf("1226/AC2: the next compose warns of an account just minted: %q", said.String()) + } + if _, held, err := inv.ModuleSecretIfIssued(ctx, "laptop", "mesh-mail", "broker-send"); err != nil || !held { + t.Fatalf("1226/AC2: the next compose carries no account for the process: held %v, %v", held, err) + } build("b-2", "bbbbbbb2222") again, _, err := inv.BusUserHash(ctx, "laptop.mesh-mail.send") if err != nil || again != hash { @@ -74,8 +85,9 @@ func TestAGainedProcessIsIssuedWhenTheDeliveryRecordsIt(t *testing.T) { } } -// 1226/AC1 and AC4 against the store: a process account nobody issued is warned of in the push's answer, naming -// issue, and the send is not refused, so the machine's other modules are composed. +// 1226/AC1 and AC4 against the store: given two modules on a machine, one with an unissued process account, when +// the machine is composed, the rest of it is composed and sent, and only the warning in the push's answer names +// the first, with `mesh-controller.issue `. func TestAnUnissuedProcessAccountIsWarnedOfAndTheRestIsComposed(t *testing.T) { open := aMesh(t) ctx := t.Context() From 181b579ac6ea3290e999cdef7d84a9617ab16213 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 21 Oct 2026 19:27:22 +0200 Subject: [PATCH 8/8] Assert the next compose carries the issued account, not a stand-in (issue 1226, review round 2) --- .../process_accounts_integration_test.go | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/cmd/mesh-controller/process_accounts_integration_test.go b/cmd/mesh-controller/process_accounts_integration_test.go index 1c2e3db5..b1a34582 100644 --- a/cmd/mesh-controller/process_accounts_integration_test.go +++ b/cmd/mesh-controller/process_accounts_integration_test.go @@ -10,8 +10,10 @@ import ( "testing" "git.novox.be/novox/mesh-sdk/go/catalogue" + "git.novox.be/novox/mesh-sdk/go/compose" "github.com/novox/mesh-controller/internal/broker" + "github.com/novox/mesh-controller/internal/inventory" "github.com/novox/mesh-controller/internal/link" "github.com/novox/mesh-controller/internal/testbus" ) @@ -75,8 +77,23 @@ func TestAGainedProcessIsIssuedWhenTheDeliveryRecordsIt(t *testing.T) { if strings.Contains(said.String(), "bus account is not issued") { t.Fatalf("1226/AC2: the next compose warns of an account just minted: %q", said.String()) } - if _, held, err := inv.ModuleSecretIfIssued(ctx, "laptop", "mesh-mail", "broker-send"); err != nil || !held { - t.Fatalf("1226/AC2: the next compose carries no account for the process: held %v, %v", held, err) + // The row the compose reads is the account issueProcess accepted (origin accepted), never a value the mesh + // made for it (a stand-in). + in, err := inputFor(ctx, open) + if err != nil { + t.Fatal(err) + } + var row *compose.OwnSecretRow + for i, r := range in.OwnSecrets { + if r.Node == "laptop" && r.Module == "mesh-mail" && r.Name == "broker-send" { + row = &in.OwnSecrets[i] + } + } + if row == nil { + t.Fatal("1226/AC2: the next compose carries no row for the process's account") + } + if row.Origin != inventory.OriginAccepted { + t.Fatalf("1226/AC2: the process's account is a stand-in the mesh made (origin %q), not the issued account", row.Origin) } build("b-2", "bbbbbbb2222") again, _, err := inv.BusUserHash(ctx, "laptop.mesh-mail.send")