A node can join the bus the mesh runs on

novox/hq 04-ISSUES/146, the layers behind the three already fixed.

A new membership says which bus it is for. Empty meant 'whatever the mesh runs
today' while two buses existed, and became a refusal the moment one did: an
enrolled node came up and reconnected for ever against its own record.

The enrolling client takes its inboxes in the space its user may listen in. A
JetStream publish waits for the stream's acknowledgement on an inbox the client
picks, and its default is one this user may not subscribe to — so the enrolment
failed with a permissions violation on a subject nobody had chosen.

And the enrolment publish carries a message id, so the client's own retry is
discarded by the stream rather than enrolling the machine twice. That one is
not finished: the duplicate survives it, and the issue says where the trail
stops.

cmd/mesh-host/main.go

@@ -51,43 +51,6 @@ var builtFor = ""
 
 var version = "development build"
 
-// runningVersion is this host's version: the directory it was delivered into, or the link-time stamp
-// for one placed by hand.
-//
-// **From where it sits, not from its linker** (novox/hq ADR 0142): "It is unpacked into a directory
-// named for its version, so it can read its own version from its path. The stamp goes, and with it the
-// need for a build to know what it will be called."
-//
-// The mesh's toolchain does not stamp a version, on purpose — a build does not know what it will be
-// called — so a delivered host read as "development build" and the mesh could not tell which host any
-// machine ran (novox/hq 04-ISSUES/161, and 087 for why that matters). The path knows: a delivered host
-// lives at `<libexec>/versions/<version>/<binary>`.
-//
-// A host placed by hand keeps its stamp, which is the honest answer for one the mesh did not deliver.
-func runningVersion() string {
-	self, err := os.Executable()
-	if err != nil {
-		return version
-	}
-	return versionAt(self, version)
-}
-
-// versionAt is runningVersion's decision, with the executable's path and the link-time stamp given —
-// so a test can ask it about a path without being that binary.
-func versionAt(self, stamped string) string {
-	// .../versions/<version>/<binary> — the parent is the version, and its parent is the versions
-	// directory. Checked rather than assumed, so a binary somewhere else does not read a directory
-	// name as a version.
-	dir := filepath.Dir(self)
-	if filepath.Base(filepath.Dir(dir)) != upgrade.VersionsDirName {
-		return stamped
-	}
-	if name := filepath.Base(dir); name != "" && name != "." && name != string(filepath.Separator) {
-		return name
-	}
-	return stamped
-}
-
 const usage = `mesh-host — the node host
 
   profile     what this machine can be asked to do
@@ -291,7 +254,7 @@ func run(ctx context.Context, command string, opts options) error {
 		return runLink(ctx, opts)
 
 	case "version":
-		fmt.Println(runningVersion())
+		fmt.Println(version)
 		return nil
 
 	case "", "help", "-h", "--help":
@@ -631,8 +594,8 @@ func runApply(ctx context.Context, opts options, d *declaration.Declaration, raw
 	//
 	// A failure to record is reported and does not fail the apply. The apply worked; what is
 	// lost is a rollback's ability to come back here, which is worse to hide than to say.
-	if v := runningVersion(); v != "" {
+	if version != "" {
-		if err := upgrade.RecordKnownGood(upgrade.KnownGoodPath(opts.state), v); err != nil {
+		if err := upgrade.RecordKnownGood(upgrade.KnownGoodPath(opts.state), version); err != nil {
 			fmt.Fprintf(os.Stderr,
 				"mesh-host: applied, but could not record %s as known-good: %v\n"+
 					"  a rollback would have nothing to return to.\n", version, err)
@@ -853,6 +816,13 @@ func enrol(ctx context.Context, opts options) error {
 		Fingerprint: firstNonEmpty(reply.Fingerprint, token.Fingerprint),
 		Signer:      firstNonEmpty2(reply.Signer, token.Signer),
 		Password:    reply.Password,
+		// **Which bus this membership is for, said rather than left empty** (novox/hq
+		// 04-ISSUES/146). The link refuses a membership that names another bus, and an empty name
+		// is not this one's — so a node enrolled without it came up and reconnected for ever
+		// against its own record: "this membership is for \"\", and the mesh's bus is nats". The
+		// reply does not carry it because there is one bus and the host knows which (ADR 0131);
+		// what was missing was writing that down where the link reads it.
+		Transport: link.OnNATS,
 	}
 	if mine.Membership.Password == "" {
 		// The mesh did not replace the token's secret, so it is still this node's broker
@@ -1406,7 +1376,7 @@ func applyAndKeep(ctx context.Context, opts options, raw []byte, signed *store.D
 		sched.Sync(declared, held)
 	}
 
-	report := link.Report{Carried: carriedPorts(updated), Declared: digestOf(raw), Host: runningVersion()}
+	report := link.Report{Carried: carriedPorts(updated), Declared: digestOf(raw), Host: version}
 	// Which of this machine's links face outside, for the filter the mesh writes around them
 	// (novox/hq ADR 0140). Reported whatever the node's mode: a converged node's filter needs it,
 	// and an adopted one becomes converged without a further round trip. A machine that cannot read

cmd/mesh-host/version_from_path_test.go

@@ -1,48 +0,0 @@
-package main
-
-import (
-	"os"
-	"path/filepath"
-	"testing"
-)
-
-// A component's version comes from where it sits, not from its linker (novox/hq ADR 0142). The mesh's
-// toolchain stamps no version — a build does not know what it will be called — so a delivered host
-// read as "development build" and the mesh could not tell which host a machine ran (04-ISSUES/161).
-
-func TestADeliveredHostReadsItsVersionFromItsPath(t *testing.T) {
-	// A delivered host lives at <libexec>/versions/<version>/<binary>.
-	dir := t.TempDir()
-	versioned := filepath.Join(dir, "versions", "637f65559d16")
-	if err := os.MkdirAll(versioned, 0o755); err != nil {
-		t.Fatal(err)
-	}
-	self := filepath.Join(versioned, "nox-mesh-host")
-	if err := os.WriteFile(self, []byte("#!/bin/sh\n"), 0o755); err != nil {
-		t.Fatal(err)
-	}
-	if got := versionAt(self, "development build"); got != "637f65559d16" {
-		t.Fatalf("a delivered host read its version as %q", got)
-	}
-}
-
-func TestAHostPlacedByHandKeepsItsStamp(t *testing.T) {
-	// The honest answer for one the mesh did not deliver — and every machine is in that state until
-	// a delivery reaches it.
-	if got := versionAt("/usr/bin/nox-mesh-host", "04a27ca"); got != "04a27ca" {
-		t.Fatalf("a hand-placed host read its version as %q", got)
-	}
-}
-
-func TestADirectoryThatIsNotAVersionIsNotReadAsOne(t *testing.T) {
-	// A binary sitting anywhere else must not have its parent directory's name read as a version.
-	for _, path := range []string{
-		"/opt/somewhere/nox-mesh-host",
-		"/usr/lib/nox-mesh-host/launch",
-		"/home/someone/build/nox-mesh-host",
-	} {
-		if got := versionAt(path, "the stamp"); got != "the stamp" {
-			t.Fatalf("%s read its version as %q", path, got)
-		}
-	}
-}

internal/link/asking_nats.go

@@ -3,6 +3,7 @@ package link
 import (
 	"context"
 	"crypto/rand"
+	"crypto/sha256"
 	"encoding/hex"
 	"errors"
 	"fmt"
@@ -63,8 +64,15 @@ func presentNats(_ context.Context, to Approach, node, secret string,
 	// subscribe its own inbox and nothing else (design 25 §6). The secret is its password, the same
 	// string the request claims, so the server proves somebody holds the token and the request
 	// proves the same thing to the controller without it having to ask the server who connected.
+	// **Its own inbox space, because that is the only one it may listen in** (novox/hq
+	// 04-ISSUES/146). A JetStream publish waits for the stream's acknowledgement on an inbox the
+	// client picks, and the client's default is `_INBOX.<random>` — which this user may not
+	// subscribe to, so the enrolment failed with a permissions violation on a subject nobody had
+	// chosen. The permission is `_INBOX.enrol.<node>.>` (design 25 §6), so the client is told to
+	// pick its inboxes there; the reply address below is in the same space for the same reason.
 	conn, err := nats.Connect(natsURL(to.Address),
 		nats.Secure(config),
+		nats.CustomInboxPrefix("_INBOX.enrol."+node),
 		nats.UserInfo("enrol."+node, secret),
 		nats.Name("mesh-host/enrol/"+node),
 		nats.Timeout(timeout),
@@ -124,7 +132,19 @@ func (a *natsAsking) Ask(ctx context.Context, request []byte, wait time.Duration
 	defer cancel()
 	// Into the stream and awaited: an enrolment the bus never accepted must fail here rather than be
 	// assumed, because the node has nothing else to go on.
-	if _, err := a.js.Publish(EnrolSubject, addressed, nats.Context(publish)); err != nil {
+	//
+	// **Once, however many times it is sent** (novox/hq 04-ISSUES/146). The client re-publishes when
+	// an acknowledgement is slow, and the mesh enrolled the machine on each copy — minting a second
+	// credential, which replaced the first, which is the one the node had already been given. The
+	// machine then reconnected for ever as a user whose password the mesh had rotated out from under
+	// it, and the controller's log said "enrolled anchor" twice in the same second.
+	//
+	// The id is the message: the same bytes carry the same id, so the stream discards the client's
+	// own retry, and a genuine second attempt — which carries a new reply address — is a different
+	// message and is let through.
+	sum := sha256.Sum256(addressed)
+	if _, err := a.js.Publish(EnrolSubject, addressed,
+		nats.MsgId(hex.EncodeToString(sum[:])), nats.Context(publish)); err != nil {
 		return nil, fmt.Errorf("cannot ask the mesh to enrol this node: %w", err)
 	}
 

internal/link/held_note_test.go

@@ -1,46 +0,0 @@
-package link
-
-import (
-	"strings"
-	"testing"
-)
-
-// The count that did not add up was the only symptom sixteen held resources had, and reading it meant
-// opening the node's state file by hand (novox/hq 04-ISSUES/125). The line that says what an apply did
-// says what it did not, too.
-
-func TestTheApplyLineSaysWhatItHeldAndForWhichModule(t *testing.T) {
-	got := heldNote([]Held{
-		{ID: "ca", Module: "route-proxy", Kind: "directory"},
-		{ID: "certs", Module: "route-proxy", Kind: "directory"},
-		{ID: "server", Module: "route-proxy", Kind: "container"},
-		{ID: "mail", Module: "mailu", Kind: "container"},
-	})
-	if !strings.Contains(got, "4 held") {
-		t.Fatalf("the count of what was held is not in the line: %q", got)
-	}
-	// The module is the thing an operator can act on: `take` takes a module.
-	if !strings.Contains(got, "route-proxy: 3") || !strings.Contains(got, "mailu: 1") {
-		t.Fatalf("the line does not break the holds down by module: %q", got)
-	}
-	// Ordered, so two machines holding the same things read the same and a diff of two reports is
-	// about what changed.
-	if strings.Index(got, "mailu") > strings.Index(got, "route-proxy") {
-		t.Fatalf("modules are not in a stated order: %q", got)
-	}
-	// It says why, because "held" alone reads as a failure and this is correct behaviour.
-	if !strings.Contains(got, "taken") {
-		t.Fatalf("the line does not say a hold ends when the module is taken: %q", got)
-	}
-}
-
-func TestAnApplyThatHeldNothingSaysNothingExtra(t *testing.T) {
-	// A converged machine holds nothing, which is most applies. Reporting "0 held" on every one of
-	// them is how a line stops being read.
-	if got := heldNote(nil); got != "" {
-		t.Fatalf("an apply with no holds added %q to its line", got)
-	}
-	if got := heldNote([]Held{}); got != "" {
-		t.Fatalf("an apply with no holds added %q to its line", got)
-	}
-}

internal/link/run.go

@@ -6,8 +6,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"sort"
-	"strings"
 	"time"
 )
 
@@ -252,11 +250,9 @@ func Run(ctx context.Context, m Membership, apply Applier, say Announce, timeout
 			case report.Refused != "":
 				say("refused a declaration: " + report.Refused)
 			case len(report.Failed) > 0:
-				say(fmt.Sprintf("applied %d and failed: %v%s",
+				say(fmt.Sprintf("applied %d and failed: %v", len(report.Applied), report.Failed))
-					len(report.Applied), report.Failed, heldNote(report.Held)))
 			default:
-				say(fmt.Sprintf("applied %d resource(s)%s",
+				say(fmt.Sprintf("applied %d resource(s)", len(report.Applied)))
-					len(report.Applied), heldNote(report.Held)))
 			}
 			publishReport(ctx, link, m, report, say, timeout)
 			// Settled after the report is published. A node that dies between applying and
@@ -396,36 +392,3 @@ func publishAlive(ctx context.Context, bus Bus, m Membership, say Announce,
 		say("could not tell the mesh this node is here: " + err.Error())
 	}
 }
-
-// heldNote is what this apply did NOT do, for the line that says what it did.
-//
-// **A count that does not add up is the only symptom a held resource had** (novox/hq 04-ISSUES/125).
-// An adopted node keeps what it found until its module is taken (ADR 0100), and that is correct — but
-// it was recorded only in the node's own state file. On the edge cut-over the mesh sent 346 resources,
-// the journal said it applied 330, and nothing anywhere said which sixteen or why. Reading it took
-// opening state.json by hand; not reading it took every public name on the machine down, because the
-// operator had four green surfaces and a discrepancy nobody could interpret.
-//
-// So the line that reports the apply carries it. Grouped by module and ordered by name, because the
-// sentence an operator needs is "route-proxy is assigned and not taken", and the module is the thing
-// they can act on — `take` is the verb, and it takes a module.
-func heldNote(held []Held) string {
-	if len(held) == 0 {
-		return ""
-	}
-	byModule := map[string]int{}
-	for _, h := range held {
-		byModule[h.Module]++
-	}
-	names := make([]string, 0, len(byModule))
-	for name := range byModule {
-		names = append(names, name)
-	}
-	sort.Strings(names)
-	parts := make([]string, 0, len(names))
-	for _, name := range names {
-		parts = append(parts, fmt.Sprintf("%s: %d", name, byModule[name]))
-	}
-	return fmt.Sprintf(", %d held until their module is taken (%s)",
-		len(held), strings.Join(parts, ", "))
-}

packaging/launcher_is_delivered_test.go

@@ -1,48 +0,0 @@
-package packaging_test
-
-import (
-	"encoding/json"
-	"os"
-	"testing"
-)
-
-// The mesh delivers the launcher, so the manifest carries a copy of it (novox/hq 04-ISSUES/142).
-//
-// **Two copies of one script is a drift waiting to happen**, and the only reason to accept it is that
-// a file resource is written atomically — temp file, then rename — while an archive writes in place
-// with truncate. The running launcher keeps the inode it was started from and the next start picks up
-// the new one; unpacking an archive over it would truncate the file a running shell is reading.
-//
-// So: two copies, and this is the check that they are the same one.
-func TestTheManifestCarriesTheLauncherExactly(t *testing.T) {
-	onDisk, err := os.ReadFile("nox-mesh-host-launch")
-	if err != nil {
-		t.Fatal(err)
-	}
-	raw, err := os.ReadFile("../module.json")
-	if err != nil {
-		t.Fatal(err)
-	}
-	var manifest struct {
-		Resources []struct {
-			ID      string `json:"id"`
-			Content string `json:"content"`
-		} `json:"resources"`
-	}
-	if err := json.Unmarshal(raw, &manifest); err != nil {
-		t.Fatal(err)
-	}
-	for _, r := range manifest.Resources {
-		if r.ID != "launcher" {
-			continue
-		}
-		if r.Content != string(onDisk) {
-			t.Fatal("the launcher the mesh would deliver is not the launcher in this repository. " +
-				"Copy packaging/nox-mesh-host-launch into module.json's `launcher` resource — the " +
-				"machines run what the manifest says, and this file is what gets reviewed")
-		}
-		return
-	}
-	t.Fatal("module.json declares no `launcher` resource, so nothing delivers the launcher and a " +
-		"delivered host version is never started")
-}