Merge pull request 'Seat dependencies (hq ADR 0207), the graphical session's seats and display provisions (ADR 0208), groups from several modules' (#264) from feat/0207-a-module-depends-on-the-seats-that-apply-its-resources into main

cmd/mesh-controller/acts.go

@@ -39,7 +39,10 @@ import (
 //
 // It costs a resolution per machine. Assignment is a person typing a command, and being told which
 // machines this just blocked is worth more than the milliseconds.
-func assign(ctx context.Context, open *stores, node, module string) (string, error) {
+func assign(ctx context.Context, open *stores, node string, modules ...string) (string, error) {
+	if len(modules) == 0 {
+		return "", fmt.Errorf("assign %s names no module", node)
+	}
 	// Held while it is recorded, so it cannot land between a converge's preview and its flip and
 	// be taken without ever having been previewed (novox/hq ADR 0100).
 	ctx, release, err := holdNodes(ctx, open, []string{node})
@@ -47,6 +50,16 @@ func assign(ctx context.Context, open *stores, node, module string) (string, err
 		return "", err
 	}
 	defer release()
+	// **The one assignment refused for what the node lacks** (novox/hq ADR 0207). Everything else
+	// an assignment leaves unresolved is kept, because assignment is not an ordering; a module whose
+	// resources are applied through a seat nothing on the node holds is refused, because that order
+	// — the service manager, the package manager and the runtime before anything that installs,
+	// runs or contains — is the mesh's to keep. Several modules in one act are judged together, so
+	// holders that depend on each other go on in one command.
+	said, err := seatDependenciesOnAssign(ctx, open, node, modules)
+	if err != nil {
+		return "", err
+	}
 	// **Before the new assignment can unsettle a seat somebody holds only by being alone**
 	// (novox/hq 04-ISSUES/170): what the mesh derived so far is written down, and then the
 	// assignment resolves against a record rather than against a coincidence.
@@ -54,65 +67,152 @@ func assign(ctx context.Context, open *stores, node, module string) (string, err
 	if err != nil {
 		return "", err
 	}
-	fresh, err := open.inventory.Assign(ctx, node, module)
-	if err != nil {
-		return "", err
+	var lines []string
+	var added []string
+	for _, module := range modules {
+		fresh, err := open.inventory.Assign(ctx, node, module)
+		if err != nil {
+			return strings.Join(lines, "\n"), err
+		}
+		if !fresh {
+			// Nothing changed, and saying "is assigned" would read as an action. One node runs one
+			// of each — the module's name is the assignment's identity (novox/hq ADR 0115).
+			lines = append(lines, fmt.Sprintf(
+				"%s already runs %s — one node runs one of each (ADR 0115); nothing changed", node, module))
+			continue
+		}
+		added = append(added, module)
+		lines = append(lines, fmt.Sprintf("%s is assigned %s", node, module))
 	}
-	if !fresh {
-		// Nothing changed, and saying "is assigned" would read as an action. One node runs one
-		// of each — the module's name is the assignment's identity (novox/hq ADR 0115).
-		return fmt.Sprintf("%s already runs %s — one node runs one of each (ADR 0115); nothing changed",
-			node, module), nil
+	if len(added) == 0 {
+		return strings.Join(lines, "\n"), nil
 	}
-	said := fmt.Sprintf("%s is assigned %s", node, module)
+	answer := strings.Join(lines, "\n")
 	for _, line := range settled {
-		said += "\n  " + line
+		answer += "\n  " + line
+	}
+	for _, line := range said {
+		answer += "\n  but " + line
 	}
 	// Its bus credential, in the same act (novox/hq issue 203): an assignment pushed before its
 	// credential exists delivers a process that cannot authenticate and crash-loops until somebody
 	// runs a second verb and a second push. Issued here when the module speaks on the bus and has
 	// no credential yet; kept when it has one, so re-assigning rotates nothing.
-	if line := issueOnAssign(ctx, open, node, module); line != "" {
-		said += "\n  " + line
+	for _, module := range added {
+		if line := issueOnAssign(ctx, open, node, module); line != "" {
+			answer += "\n  " + line
+		}
 	}
 	plan, _, err := planFor(ctx, open, node)
 	if err != nil {
 		// Kept, and still refused. Both halves are the answer, and the rest of the mesh is still
 		// worth reporting: this machine's refusal is rarely the only consequence.
-		return said + blockedElsewhere(ctx, open, node), err
+		return answer + blockedElsewhere(ctx, open, node), err
 	}
 	// Kept, and cannot be hosted here. Said at once rather than discovered at push: a module whose
 	// capability the machine lacks is on the wrong machine, and the assignment records what a person
 	// meant while this line says it will not run until it moves. The rest of the node still pushes.
+	isAdded := map[string]bool{}
+	for _, m := range added {
+		isAdded[m] = true
+	}
 	for _, u := range plan.Unhostable {
-		if u.Module != module {
+		if !isAdded[u.Module] {
 			continue
 		}
 		for _, c := range u.Missing {
-			said += "\n  but " + catalogue.WrongMachine(u.Module, c, node)
+			answer += "\n  but " + catalogue.WrongMachine(u.Module, c, node)
 		}
 	}
-	return said + fmt.Sprintf("\n  run `push %s` to send it", node) +
+	return answer + fmt.Sprintf("\n  run `push %s` to send it", node) +
 		blockedElsewhere(ctx, open, node), nil
 }
 
-// unassign takes a module off a node. What it leaves behind is the host's business: a directory
+// seatDependenciesOnAssign is the refusal ADR 0207 makes at assignment, or the lines an assignment
+// says beside itself when a dependency has no holder in the catalogue to name. Modules already
+// assigned are not new and are not judged again.
+func seatDependenciesOnAssign(ctx context.Context, open *stores, node string, modules []string) ([]string, error) {
+	shelf, err := open.inventory.Catalogue(ctx)
+	if err != nil {
+		return nil, err
+	}
+	assigned, err := open.inventory.Assigned(ctx, node)
+	if err != nil {
+		return nil, err
+	}
+	already := map[string]bool{}
+	for _, a := range assigned {
+		already[a] = true
+	}
+	var adding []string
+	for _, m := range modules {
+		if !already[m] {
+			adding = append(adding, m)
+		}
+	}
+	return catalogue.AssignRefusal(shelf, node, assigned, adding)
+}
+
+// unassign takes modules off a node. What they leave behind is the host's business: a directory
 // holding anything the mesh did not put there is kept (novox/hq ADR 0030).
 //
 // It reports the rest of the mesh for the same reason assign does, and more sharply: taking a
 // module off one machine is the ordinary way to stop providing something to another, and nothing
 // about the command's own output would ever have said so.
-func unassign(ctx context.Context, open *stores, node, module string) (string, error) {
+//
+// **Refused when it takes away the last holder of a seat a module left on the node depends on**
+// (novox/hq ADR 0207) — the other side of refusing that module's assignment without one. Several
+// modules in one act are judged together, so a holder and its dependents come off in one command.
+func unassign(ctx context.Context, open *stores, node string, modules ...string) (string, error) {
+	if len(modules) == 0 {
+		return "", fmt.Errorf("unassign %s names no module", node)
+	}
 	ctx, release, err := holdNodes(ctx, open, []string{node})
 	if err != nil {
 		return "", err
 	}
 	defer release()
-	if err := open.inventory.Unassign(ctx, node, module); err != nil {
+	shelf, err := open.inventory.Catalogue(ctx)
+	if err != nil {
 		return "", err
 	}
+	assigned, err := open.inventory.Assigned(ctx, node)
+	if err != nil {
+		return "", err
+	}
+	// Every one checked before any is taken off, so a refusal leaves the node as it was.
+	runs := map[string]bool{}
+	for _, a := range assigned {
+		runs[a] = true
+	}
+	for _, module := range modules {
+		if !runs[module] {
+			return "", fmt.Errorf("%s is not assigned to %s", module, node)
+		}
+	}
+	if err := catalogue.UnassignRefusal(shelf, node, assigned, modules); err != nil {
+		return "", err
+	}
+	for _, module := range modules {
+		if err := open.inventory.Unassign(ctx, node, module); err != nil {
+			return "", err
+		}
+	}
 	return fmt.Sprintf("%s no longer runs %s — run `push %s` to make it so",
-		node, module, node) + blockedElsewhere(ctx, open, node), nil
+		node, strings.Join(modules, ", "), node) + blockedElsewhere(ctx, open, node), nil
+}
+
+// splitModules is a surface's one `module` field as the modules it names: several, comma-separated,
+// are one act (novox/hq ADR 0207), so the holders that depend on each other go on together from the
+// command API and the controller seat's verbs as they do from the command line.
+func splitModules(field string) []string {
+	var out []string
+	for _, m := range strings.Split(field, ",") {
+		if m = strings.TrimSpace(m); m != "" {
+			out = append(out, m)
+		}
+	}
+	return out
 }
 
 // blockedElsewhere is every OTHER machine that cannot be worked out as things now stand.

cmd/mesh-controller/api.go

@@ -95,10 +95,10 @@ func commands(who Authenticator) http.Handler {
 	mux := http.NewServeMux()
 
 	mux.HandleFunc("POST /assign", acting(who, true, func(ctx context.Context, open *stores, in request) (string, error) {
-		return assign(ctx, open, in.Node, in.Module)
+		return assign(ctx, open, in.Node, splitModules(in.Module)...)
 	}))
 	mux.HandleFunc("POST /unassign", acting(who, true, func(ctx context.Context, open *stores, in request) (string, error) {
-		return unassign(ctx, open, in.Node, in.Module)
+		return unassign(ctx, open, in.Node, splitModules(in.Module)...)
 	}))
 	// Adoption (novox/hq ADR 0100): the same acts as `take`, `converge` and `adopt`.
 	mux.HandleFunc("POST /take", acting(who, true, func(ctx context.Context, open *stores, in request) (string, error) {

cmd/mesh-controller/build.go

@@ -650,6 +650,11 @@ type answers struct {
 	// public name on the machine went dark. The holds were correct; they were recorded only in the
 	// machine's own state file, and the one visible symptom was a count that did not add up.
 	untaken map[string]map[string]int
+	// unheld is every module on a machine whose resources are applied through a seat nothing on
+	// that machine holds (novox/hq ADR 0207), with the modules that could hold it. Reported, not
+	// refused, until the switch — and while there is any, the mesh is not all well: the order the
+	// machines' modules are built in is the mesh's to keep, and this is where it says it is not kept.
+	unheld []catalogue.Unheld
 }
 
 // heldBy is every artifact this mesh has built, for a build that may need one as its base.

cmd/mesh-controller/main.go

@@ -179,8 +179,8 @@ func usage() {
   seat <name> --to <node>/<module>   hand a seat to that assignment as one act; never empty in between (ADR 0131)
   board [--listen ADDR]          the same three questions, as a page that holds nothing
   api --issuer URL [--listen A]  assign and unassign over http, for a surface that is not here
-  assign <node> <module>         put a module on a node
-  unassign <node> <module>       take it off
+  assign <node> <module>...      put modules on a node, judged together (ADR 0207)
+  unassign <node> <module>...    take them off
   take <node> <module>           preview a module's cutover on an adopted node: what runs beside
                                  what it declares; --yes <digest> cuts it over as previewed
   converge <node> [--yes <digest>] [--filter nftables]   preview, then make, an adopted node converged

cmd/mesh-controller/modules.go

@@ -334,8 +334,10 @@ func moduleCommand(ctx context.Context, args []string) error {
 }
 
 func assignCommand(ctx context.Context, verb string, args []string) error {
-	if len(args) != 2 {
-		return fmt.Errorf("%s <node> <module>", verb)
+	// Several modules in one act (novox/hq ADR 0207): holders that depend on each other — the
+	// service manager and the package manager — can only go on, or come off, together.
+	if len(args) < 2 {
+		return fmt.Errorf("%s <node> <module> [<module>…]", verb)
 	}
 	open, err := openStores(ctx)
 	if err != nil {
@@ -349,7 +351,7 @@ func assignCommand(ctx context.Context, verb string, args []string) error {
 	if verb == "unassign" {
 		act = unassign
 	}
-	said, err := act(ctx, open, args[0], args[1])
+	said, err := act(ctx, open, args[0], args[1:]...)
 	if said != "" {
 		fmt.Println(said)
 	}

cmd/mesh-controller/plan.go

@@ -10,6 +10,7 @@ import (
 	"os"
 	"sort"
 	"strings"
+	"sync"
 
 	"github.com/novox/mesh-controller/internal/broker"
 	"github.com/novox/mesh-controller/internal/catalogue"
@@ -127,6 +128,7 @@ func planFor(ctx context.Context, open *stores, nodeName string) (catalogue.Reso
 		// a mesh-wide gatherer may pass over — see notResolvable.
 		return catalogue.Resolution{}, nil, notResolvable{err}
 	}
+	logUnheld(nodeName, resolved.Unheld)
 
 	// The credential for each thing this node takes from elsewhere. Made once and kept, so the
 	// password a provider is told to create is the one its consumer was given — and sealed to
@@ -1325,3 +1327,34 @@ func providerModuleOf(resolved catalogue.Resolution, open *stores, ctx context.C
 	}
 	return ""
 }
+
+// unheldLogged is what was last logged about each node's unmet seat dependencies, so the log says
+// each change once (novox/hq ADR 0207), on stderr so `status --json` stays a document — planFor runs for every status, push and assignment, and a
+// line per call would bury the one that changed.
+var (
+	unheldLogged   = map[string]string{}
+	unheldLoggedMu sync.Mutex
+)
+
+// logUnheld logs a node's unmet seat dependencies when they differ from what was last logged for
+// it, including when they become none.
+func logUnheld(node string, unheld []catalogue.Unheld) {
+	lines := make([]string, 0, len(unheld))
+	for _, u := range unheld {
+		lines = append(lines, u.String())
+	}
+	now := strings.Join(lines, "\n")
+	unheldLoggedMu.Lock()
+	before, seen := unheldLogged[node]
+	unheldLogged[node] = now
+	unheldLoggedMu.Unlock()
+	if (seen && before == now) || (!seen && now == "") {
+		return
+	}
+	if now == "" {
+		fmt.Fprintf(os.Stderr, "%s: every seat its modules depend on is held (novox/hq ADR 0207)\n", node)
+		return
+	}
+	fmt.Fprintf(os.Stderr, "%s: %d unmet seat dependenc(ies), reported and not refused (novox/hq ADR 0207):\n  %s\n",
+		node, len(lines), strings.Join(lines, "\n  "))
+}

cmd/mesh-controller/readable.go

@@ -3,6 +3,7 @@ package main
 import (
 	"encoding/json"
 	"fmt"
+	"github.com/novox/mesh-controller/internal/catalogue"
 	"github.com/novox/mesh-controller/internal/inventory"
 	"sort"
 	"time"
@@ -73,6 +74,10 @@ type meshStatus struct {
 	// alone. A document without this called a machine well while a predecessor's chain refused
 	// what the mesh declared open.
 	Filtered []machineFiltered `json:"filtered,omitempty"`
+	// Unheld is every module on a machine whose resources are applied through a seat nothing on
+	// that machine holds, with the modules that could hold it (novox/hq ADR 0207). Absent when every
+	// dependency is met. Reported, not refused, until the switch.
+	Unheld []catalogue.Unheld `json:"unheld,omitempty"`
 }
 
 // machineFiltered is one rule set on a converged machine that the mesh did not write and that
@@ -204,6 +209,7 @@ func statusAsJSON(asked answers) ([]byte, error) {
 			out.Filtered = append(out.Filtered, machineFiltered{Node: name, Where: x.Where, Owner: x.Owner, Refuses: x.Refuses})
 		}
 	}
+	out.Unheld = asked.unheld
 	for name := range asked.refused {
 		out.Unresolved = append(out.Unresolved, machineUnresolved{
 			Node: name, Problem: asked.refused[name]})

cmd/mesh-controller/seat_dependencies_test.go

@@ -0,0 +1,147 @@
+package main
+
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+
+	"github.com/novox/mesh-controller/internal/catalogue"
+)
+
+// Defends novox/hq ADR 0207 at the controller's acts: `assign` refuses a module whose resources a
+// seat nothing on the node holds applies, `unassign` refuses taking the last holder from under its
+// dependents, and `status` reports what composition does not yet refuse.
+
+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"}}},
+		Resources: []map[string]any{{"id": "systemd", "type": "package", "package": "systemd"}}}
+}
+
+func packageManagerHolder() catalogue.Manifest {
+	return catalogue.Manifest{Module: "pacman", Version: "1",
+		Claims:    []catalogue.Claim{{Name: catalogue.PackageManagerSeat, Scope: catalogue.ScopeNode}},
+		Resources: []map[string]any{{"id": "refresh", "type": "service", "unit": "pacman-refresh.timer"}}}
+}
+
+func aDaemon() catalogue.Manifest {
+	return catalogue.Manifest{Module: "sshd", Version: "1",
+		Resources: []map[string]any{{"id": "sshd", "type": "service", "unit": "sshd.service"}}}
+}
+
+func TestAnAssignmentWithoutItsHolderIsRefusedAndNotKept(t *testing.T) {
+	open := aMesh(t)
+	ctx := t.Context()
+	register(t, open, serviceManagerHolder())
+	register(t, open, packageManagerHolder())
+	register(t, open, aDaemon())
+
+	_, err := assign(ctx, open, "laptop", "sshd")
+	if err == nil {
+		t.Fatal("sshd went onto a machine nothing holds the service manager of")
+	}
+	for _, want := range []string{catalogue.ServiceManagerSeat, "systemd", "ADR 0207"} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("the refusal does not say %q:\n%v", want, err)
+		}
+	}
+	assigned, err := open.inventory.Assigned(ctx, "laptop")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if contains(assigned, "sshd") {
+		t.Fatalf("a refused assignment was kept: %v", assigned)
+	}
+
+	// The holders depend on each other, so neither goes on alone — and both go on in one act.
+	if _, err := assign(ctx, open, "laptop", "systemd"); err == nil {
+		t.Fatal("systemd went on alone though its package needs a package manager")
+	}
+	if said, err := assign(ctx, open, "laptop", "systemd", "pacman"); err != nil {
+		t.Fatalf("the two holders assigned together were refused: %v\n%s", err, said)
+	}
+	if said, err := assign(ctx, open, "laptop", "sshd"); err != nil {
+		t.Fatalf("sshd beside its holder was refused: %v\n%s", err, said)
+	}
+}
+
+func TestTheControllerSeatsAssignTakesSeveralModulesAsOneAct(t *testing.T) {
+	argv, err := argvFor("assign", map[string]any{"node": "laptop", "module": "systemd, pacman"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if strings.Join(argv, " ") != "assign laptop systemd pacman" {
+		t.Errorf("the seat's assign became %v", argv)
+	}
+}
+
+func TestUnassigningTheLastHolderUnderItsDependentsIsRefused(t *testing.T) {
+	open := aMesh(t)
+	ctx := t.Context()
+	register(t, open, serviceManagerHolder())
+	register(t, open, packageManagerHolder())
+	register(t, open, aDaemon())
+	if _, err := assign(ctx, open, "laptop", "systemd", "pacman", "sshd"); err != nil {
+		t.Fatal(err)
+	}
+
+	_, err := unassign(ctx, open, "laptop", "systemd")
+	if err == nil {
+		t.Fatal("the service manager came off a machine still running services")
+	}
+	for _, want := range []string{catalogue.ServiceManagerSeat, "sshd", "pacman"} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("the refusal does not name %q:\n%v", want, err)
+		}
+	}
+	assigned, _ := open.inventory.Assigned(ctx, "laptop")
+	if !contains(assigned, "systemd") {
+		t.Fatalf("a refused unassignment took the module off anyway: %v", assigned)
+	}
+	if _, err := unassign(ctx, open, "laptop", "sshd"); err != nil {
+		t.Fatalf("a dependent could not come off: %v", err)
+	}
+}
+
+func TestStatusReportsAnUnheldDependencyWithoutRefusingTheMachine(t *testing.T) {
+	open := aMesh(t)
+	ctx := t.Context()
+	register(t, open, serviceManagerHolder())
+	register(t, open, aDaemon())
+	// Assigned straight into the store: a machine whose modules predate the rule, which is every
+	// machine on the day it ships.
+	if _, err := open.inventory.Assign(ctx, "laptop", "sshd"); err != nil {
+		t.Fatal(err)
+	}
+
+	asked, err := theThreeQuestions(ctx, open)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, refused := asked.refused["laptop"]; refused {
+		t.Fatalf("an unmet dependency refused the machine before the switch: %s", asked.refused["laptop"])
+	}
+	if asked.well() {
+		t.Error("a mesh with an unheld dependency reads as all well")
+	}
+	got := printed(t, func() error { return printStatus(asked) })
+	for _, want := range []string{"unheld", "laptop", "sshd", catalogue.ServiceManagerSeat, "systemd"} {
+		if !strings.Contains(got, want) {
+			t.Errorf("status does not say %q:\n%s", want, got)
+		}
+	}
+	body, err := statusAsJSON(asked)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var doc struct {
+		Unheld []catalogue.Unheld `json:"unheld"`
+	}
+	if err := json.Unmarshal(body, &doc); err != nil {
+		t.Fatal(err)
+	}
+	if len(doc.Unheld) != 1 || doc.Unheld[0].Module != "sshd" || doc.Unheld[0].Seat != catalogue.ServiceManagerSeat {
+		t.Errorf("the document's unheld is %+v", doc.Unheld)
+	}
+}

cmd/mesh-controller/seatverbs.go

@@ -113,7 +113,9 @@ func argvFor(verb string, args map[string]any) ([]string, error) {
 		if err := need("node", "module"); err != nil {
 			return nil, err
 		}
-		return []string{verb, str("node"), str("module")}, nil
+		// Several modules comma-separated, judged as one act (novox/hq ADR 0207): the holders of
+		// the seats that apply resources depend on each other and go on together.
+		return append([]string{verb, str("node")}, splitModules(str("module"))...), nil
 	case "pin":
 		if err := need("node", "provision", "from", "module"); err != nil {
 			return nil, err

cmd/mesh-controller/status.go

@@ -282,6 +282,22 @@ func printStatus(asked answers) error {
 		fmt.Printf("\n  `take <node> <module>` compares what runs against what it declares, and runs it\n\n")
 	}
 
+	if len(asked.unheld) > 0 {
+		// **Reported, and not refused yet** (novox/hq ADR 0207 §4). Each machine still resolves and
+		// is sent what it would be; this says which of its modules depend on a seat nothing there
+		// holds, until every machine has its holders and the switch makes it a refusal.
+		fmt.Printf("%d module dependenc(ies) on a seat nothing on the machine holds (unheld, ADR 0207):\n",
+			len(asked.unheld))
+		for _, u := range asked.unheld {
+			holders := "no module in the catalogue claims it yet"
+			if len(u.Holders) > 0 {
+				holders = "could be held by " + strings.Join(u.Holders, ", ")
+			}
+			fmt.Printf("  %-12s %-24s %-24s %s\n", u.Node, u.Module, u.Seat, holders)
+		}
+		fmt.Printf("\n  `assign <node> <holder>` meets it; reported until every machine has its holders, then refused\n\n")
+	}
+
 	if adopted := adoptedNodes(nodes); len(adopted) > 0 {
 		// Said, because nothing forces the flip: a node left adopted is visible here rather than
 		// read as converged (novox/hq ADR 0100). Not a fault, so it does not break "all well".
@@ -386,6 +402,20 @@ func theThreeQuestions(ctx context.Context, open *stores) (answers, error) {
 	if err != nil {
 		return answers{}, err
 	}
+	// And which machines run a module whose resources a seat nothing there holds applies (novox/hq
+	// ADR 0207). Each machine resolved again rather than threaded through whoResolves, whose answer
+	// the private network is built from and should say nothing else; a machine that does not
+	// resolve is already in refused, and is passed over here.
+	for _, n := range out.nodes {
+		plan, _, err := planFor(ctx, open, n.Name)
+		if err != nil {
+			if unresolvable(err) {
+				continue
+			}
+			return answers{}, err
+		}
+		out.unheld = append(out.unheld, plan.Unheld...)
+	}
 	out.plans, err = inv.RecentPlans(ctx, 5)
 	if err != nil {
 		return answers{}, err
@@ -496,7 +526,7 @@ func untakenModules(ctx context.Context, inv *inventory.Inventory, nodes []inven
 func (a answers) well() bool {
 	return len(a.wrong) == 0 && len(a.quiet) == 0 && len(a.behind) == 0 &&
 		len(a.waiting) == 0 && len(a.refused) == 0 && a.network == "" && len(a.untaken) == 0 &&
-		len(a.filtered) == 0
+		len(a.filtered) == 0 && len(a.unheld) == 0
 }
 
 // hostSplit is which machines report which host version, for every version more than one machine