Offer no Release on a machine that also holds a person's own hold (issue 1732, review round 1)

cmd/mesh-controller/held_moves.go

@@ -32,7 +32,7 @@ type heldFacts struct {
 	moves []heldMove
 }
 
-// gatherHeldMoves is, per machine, the moves that wait for a person and are no person's own choice, sorted by machine.
+// gatherHeldMoves is, per machine that holds at least one move no person chose to hold, all its held moves, by machine.
 func gatherHeldMoves(ctx context.Context, inv *inventory.Inventory) ([]heldFacts, error) {
 	nodes, err := inv.Nodes(ctx)
 	if err != nil {
@@ -48,23 +48,34 @@ func gatherHeldMoves(ctx context.Context, inv *inventory.Inventory) ([]heldFacts
 		if err != nil {
 			return nil, err
 		}
-		var asked []heldMove
-		for _, m := range moves {
-			if !m.Chosen {
-				asked = append(asked, m)
-			}
-		}
-		if len(asked) > 0 {
-			out = append(out, heldFacts{node: n.Name, moves: asked})
+		h := heldFacts{node: n.Name, moves: moves}
+		if len(h.asked()) > 0 {
+			out = append(out, h)
 		}
 	}
 	return out, nil
 }
 
-// modules are the held moves' modules, in order.
-func (h heldFacts) modules() []string {
-	out := make([]string, 0, len(h.moves))
+// asked are the moves no person chose to hold, and chosen the ones a person did (a module set to record with a why, the bus).
+func (h heldFacts) asked() []heldMove  { return h.byChoice(false) }
+func (h heldFacts) chosen() []heldMove { return h.byChoice(true) }
+
+func (h heldFacts) byChoice(chosen bool) []heldMove {
+	var out []heldMove
 	for _, m := range h.moves {
+		if m.Chosen == chosen {
+			out = append(out, m)
+		}
+	}
+	return out
+}
+
+// modules are the held moves' modules, in order, of all of them.
+func (h heldFacts) modules() []string { return moduleNames(h.moves) }
+
+func moduleNames(moves []heldMove) []string {
+	out := make([]string, 0, len(moves))
+	for _, m := range moves {
 		out = append(out, m.Module)
 	}
 	return out
@@ -72,19 +83,21 @@ func (h heldFacts) modules() []string {
 
 // releaseAction is the one answer a held move offers: the push of the machine, approved by the operator. The push
 // names every module whose move waits there as one that may move its data: the operator's Approve is the word
-// ADR 0217 asks for, and the ask says so.
+// ADR 0217 asks for, and the ask says so. Offered only where no person's own hold is on the machine too: a push
+// composes every recorded module at its current build, so it would send that move as well, which the ask never named
+// (ADR 0259 §6), and hold the whole machine if that module's data would move.
 func (h heldFacts) releaseAction() conditions.Action {
 	return conditions.Action{Label: "Release", Verb: "mesh-controller.push", Level: conditions.LevelApprove,
-		Arguments: map[string]string{"node": h.node, "move": strings.Join(h.modules(), ","), "why": "",
+		Arguments: map[string]string{"node": h.node, "move": strings.Join(moduleNames(h.asked()), ","), "why": "",
 			"cause": conditions.CauseOperatorAnswer}}
 }
 
 // heldWords are the plain words of a machine's held moves: which modules wait, why in general, and what an Approve does.
 func heldWords(h heldFacts) (headline, explanation, needs string, resolved string) {
-	names := namesWords(h.modules(), 3)
+	names := namesWords(moduleNames(h.asked()), 3)
 	headline = clipWords(conditions.Capital(names)+" waits for you on "+h.node, conditions.HeadlineMax)
 	recreates, data := false, false
-	for _, m := range h.moves {
+	for _, m := range h.asked() {
 		switch {
 		case strings.Contains(m.Why, "recreates"):
 			recreates = true
@@ -105,6 +118,15 @@ func heldWords(h heldFacts) (headline, explanation, needs string, resolved strin
 		"it waits for you. Release sends %s everything it is owed, these among it, and lets their data move where it "+
 		"has to. Nothing is sent until you answer.", names, why, h.node, h.node), conditions.ExplanationMax)
 	needs = "release them " + FromMeshMCPServer
+	if chosen := h.chosen(); len(chosen) > 0 {
+		// A push of the machine would send the move a person chose to hold as well: no answer is offered here.
+		explanation = clipWords(fmt.Sprintf("The new version of %s %s on %s, so it does not go out with its delivery; "+
+			"it waits for you. %s is also held there by your own choice, and a push of %s would send that too, so "+
+			"nothing is offered here: the push is yours at the controller's terminal.",
+			names, why, h.node, conditions.Capital(namesWords(moduleNames(chosen), 3)), h.node), conditions.ExplanationMax)
+		needs = fmt.Sprintf("push %s at the controller's terminal, once you have decided about %s.", h.node,
+			namesWords(moduleNames(chosen), 3))
+	}
 	resolved = "Resolved: " + names + " is no longer waiting on " + h.node
 	return
 }
@@ -119,13 +141,20 @@ func watchMovesHeld(f *signalFacts) []conditions.Observation {
 			said = append(said, fmt.Sprintf("%s %s → %s %s", m.Module, short(m.From), short(m.To), m.Why))
 		}
 		headline, explanation, needs, resolved := heldWords(h)
+		if c := h.chosen(); len(c) > 0 {
+			said = append(said, "held by a person's own choice: "+strings.Join(moduleNames(c), ", "))
+		}
+		var actions []conditions.Action
+		if len(h.chosen()) == 0 {
+			actions = []conditions.Action{h.releaseAction()}
+		}
 		out = append(out, conditions.Observation{Scope: conditions.ScopeMachine, ID: h.node, Kind: kindMovesHeld,
 			Machine: h.node, Severity: conditions.Warning, Resolver: conditions.ResolverOperator,
 			Summary: fmt.Sprintf("%s has %d move(s) of recorded modules waiting for a person's push: %s — `push %s` sends them",
 				h.node, len(h.moves), strings.Join(h.modules(), ", "), h.node),
 			Said:     strings.Join(said, "; "),
 			Headline: headline, Explanation: explanation, Needs: needs, Resolved: resolved,
-			Actions: []conditions.Action{h.releaseAction()}})
+			Actions: actions})
 	}
 	return out
 }

cmd/mesh-controller/held_moves_integration_test.go

@@ -59,7 +59,7 @@ func TestAHoldAPersonChoseIsNotAsked(t *testing.T) {
 	}
 	held, err := gatherHeldMoves(t.Context(), rm.open.inventory)
 	if err != nil || len(held) != 0 {
-		t.Fatalf("a person's own hold is asked: %+v (%v)", held, err)
+		t.Fatalf("a machine holding only a person's own hold is asked: %+v (%v)", held, err)
 	}
 	if moves, _ := heldMovesOn(t.Context(), rm.open.inventory, mustMoveFacts(t, rm), "anchor"); len(moves) != 1 || !moves[0].Chosen {
 		t.Fatalf("status still names it, marked as chosen: %+v", moves)

cmd/mesh-controller/held_moves_test.go

@@ -206,3 +206,32 @@ func TestAPlanWaitingForAGateBehindAFailedReleaseIsNotLate(t *testing.T) {
 		t.Errorf("a plan held behind a failed release reads %q", line)
 	}
 }
+
+// 1732/AC10: a machine that also holds a move a person chose to hold gets no Release, since a push would send that move
+// too; the condition names both and says the push is the controller's terminal. A machine with only asked moves keeps it.
+func TestAMachineAlsoHoldingAPersonsOwnHoldIsOfferedNoRelease(t *testing.T) {
+	f := calm(time.Date(2026, 10, 21, 17, 30, 0, 0, time.UTC))
+	h := aHeldMailu()
+	h.moves = append(h.moves, heldMove{Module: "nftables", From: "n1aaaa", To: "n2bbbb",
+		Why: "waits: a person set it to record, and holds every move of it until they push", Chosen: true})
+	f.held = []heldFacts{h}
+	got := watchMovesHeld(f)
+	if len(got) != 1 || len(got[0].Actions) != 0 {
+		t.Fatalf("%+v", got)
+	}
+	o := got[0]
+	text := o.Headline + o.Explanation + o.Needs + o.Summary + o.Said
+	for _, want := range []string{"mailu", "nftables", "terminal", "push novox"} {
+		if !strings.Contains(text, want) {
+			t.Errorf("does not say %q: %q", want, text)
+		}
+	}
+	w := conditions.Words{Headline: o.Headline, Explanation: o.Explanation, Resolved: o.Resolved, Needs: o.Needs}
+	if why, ok := conditions.PlainWords(w, "novox"); !ok {
+		t.Errorf("not plain: %s", why)
+	}
+	f.held = []heldFacts{aHeldMailu()}
+	if got := watchMovesHeld(f); len(got) != 1 || len(got[0].Actions) != 1 || got[0].Actions[0].Arguments["move"] != "mailu" {
+		t.Fatalf("a machine with only asked moves: %+v", got)
+	}
+}