Merge pull request 'Node-engine: act on a backup answer whatever runs, say why a module stays held, and show the want and the hold in the composition verb (issue 1585)' (#208) from fix/1585-held-answer into main

internal/agent/agent.go

@@ -1305,20 +1305,35 @@ func (a *Agent) settleOne(ctx context.Context, module string) {
 		a.mu.Unlock()
 	}
 	a.mu.Lock()
-	if r, ok := a.running[module]; ok && r.FP == w.Want.FP && r.Composed == w.Want.Composed && r.Phase != delivery.PhaseRemoved {
+	r, runs := a.running[module]
+	runsWant := runs && r.FP == w.Want.FP && r.Composed == w.Want.Composed && r.Phase != delivery.PhaseRemoved
+	att, tried := a.attempt[module]
+	same := tried && att.FP == w.Want.FP && att.Composed == w.Want.Composed
+	if same && att.Held {
+		// Held for a failed backup: looked at before what runs, since a hold may name what the want names (its have kept
+		// the composition it held, nats on novox, 2026-11-04), and an answer in a later revision of the want would then
+		// be swallowed by "it runs what its want names" (issue 1585, the second failure). The operator's answer, again or
+		// skip, in a revision after the one the hold took, is acted on whatever runs; any other new revision of the want
+		// is the answer only for a move (ADR 0568 §4), as before.
+		answered := w.Rev > att.AnsweredRev && (w.Want.Backup == delivery.BackupAgain || w.Want.Backup == delivery.BackupSkip)
+		if !answered && (runsWant || (att.AnsweredRev >= w.Rev && att.CompRev == comp.Rev)) {
+			a.sayOnce("held:"+module, fmt.Sprintf("%s is held for a failed backup before %s: it waits for the operator's "+
+				"answer, and its want (revision %d) carries none since the hold (revision %d)", module, w.Want.FP, w.Rev, att.AnsweredRev))
+			a.mu.Unlock()
+			return
+		}
+		if answered {
+			a.sayOnce("answered:"+module+":"+fmt.Sprint(w.Rev), fmt.Sprintf("%s is held for a failed backup, and "+
+				"its want (revision %d) carries the operator's answer %q: acted on now", module, w.Rev, w.Want.Backup))
+		}
+	} else if runsWant {
 		a.mu.Unlock()
 		return
 	}
-	att, tried := a.attempt[module]
-	same := tried && att.FP == w.Want.FP && att.Composed == w.Want.Composed
 	if same && !att.Held && att.WantRev == w.Rev && att.CompRev == comp.Rev {
 		a.mu.Unlock()
 		return // failed already for exactly these keys: retried by the next change of either
 	}
-	if same && att.Held && att.AnsweredRev >= w.Rev && att.CompRev == comp.Rev {
-		a.mu.Unlock()
-		return // held for a failed backup, and no answer since
-	}
 	if module == NodeModule {
 		// Not while a module's composition cannot be read or is refused: what it carries may be what left the node's own,
 		// and the node's own apply would remove it, held out by nothing (issue 1945, the review's gap 2).
@@ -1390,8 +1405,12 @@ func (a *Agent) baseline(ctx context.Context, module string, w wantNow, comp com
 			}
 			if h.Phase == string(delivery.PhaseHeld) {
 				// Held for a failed backup, and the operator's answer is still to come: the hold is kept, nothing is
-				// backed up again or applied, and the next revision of the want is the answer (ADR 0568 §4).
+				// backed up again or applied, and the next revision of the want is the answer (ADR 0568 §4). Said, so
+				// why it holds is read from the journal and not guessed (issue 1585: this branch was silent).
 				a.mu.Lock()
+				a.sayOnce("held:"+module, fmt.Sprintf("%s is held for a failed backup (%s) at this start: it keeps what "+
+					"its have names, %s, and waits for the operator's answer; its want (revision %d) carries none",
+					module, h.Detail, short(h.Composed), w.Rev))
 				a.attempt[module] = attempt{FP: w.Want.FP, Composed: w.Want.Composed, WantRev: w.Rev, CompRev: comp.Rev, Held: true,
 					AnsweredRev: w.Rev}
 				if h.Composed != "" {

internal/agent/agent_test.go

@@ -636,6 +636,78 @@ func TestADataModuleIsBackedUpFirstAndHeldWhenItFails(t *testing.T) {
 		}
 		stopR()
 	}
+
+	// The live shape (nats on novox, 2026-11-04, issue 1585, the second failure): a module the node composes itself, of
+	// policy record, its want Self with the yes for its composition, and its held have naming that very composition (what
+	// runs is what the want names). The operator's answer acted on at the start, and in a revision of the want that
+	// arrives while the module is held; the engine's read tool says the want's answer and the hold.
+	for _, row := range []struct {
+		name      string
+		atStart   string // the answer the want carries when the engine starts
+		later     string // the answer a later revision carries; "" writes none
+		backups   int
+		holdsTill bool // it is held, and said held, before the later revision
+	}{
+		{"answered before the start", delivery.BackupAgain, "", 1, false},
+		{"answered while held", "", delivery.BackupAgain, 1, true},
+		{"skipped while held", "", delivery.BackupSkip, 0, true},
+	} {
+		t.Run(row.name, func(t *testing.T) {
+			r := newBed(t)
+			r.env.SelfComposed, r.kept = true, nil
+			r.env.Policy = func(context.Context, string) string { return catalogue.PolicyRecord }
+			ck, craw, dig := composition(t, "nats", fp2, true, file("nats", "x", "2"))
+			r.comps.before(ck, craw)
+			wk, _ := delivery.WantKey(node, "nats")
+			selfWant := func(backup string) []byte {
+				raw, err := delivery.EncodeWant(delivery.Want{FP: fp2, Self: true, Yes: dig, Backup: backup})
+				if err != nil {
+					t.Fatal(err)
+				}
+				return raw
+			}
+			r.wants.before(wk, selfWant(row.atStart))
+			r.haves.stored = map[string]Have{"nats": {Module: "nats", FP: fp2, Composed: dig, Phase: string(delivery.PhaseHeld),
+				Detail: "the backup failed: sh: exit status 1"}}
+			stopR := r.run()
+			defer stopR()
+			if row.holdsTill {
+				eventually(t, "the hold at the start is not said", func() bool {
+					r.mu.Lock()
+					defer r.mu.Unlock()
+					for _, s := range r.said {
+						if strings.Contains(s, "nats is held for a failed backup") && strings.Contains(s, "carries none") {
+							return true
+						}
+					}
+					return false
+				})
+				view, err := r.ag.Composition(context.Background(), "nats", "", false)
+				if err != nil || view.Attempt == nil || !view.Attempt.Held || view.Want == nil || view.Want.Backup != "" || !view.Started {
+					t.Fatalf("the read tool does not say the hold: %+v %+v %v", view.Attempt, view.Want, err)
+				}
+				if got := r.appliedList(); len(got) != 0 {
+					t.Fatalf("applied behind a hold with no answer: %v", got)
+				}
+				r.wants.put(wk, selfWant(row.later))
+				eventually(t, "the read tool does not say the answer", func() bool {
+					v, err := r.ag.Composition(context.Background(), "nats", "", false)
+					return err == nil && v.Want != nil && v.Want.Backup == row.later && v.Want.Revision > view.Want.Revision
+				})
+			}
+			eventually(t, "the answer was not acted on", func() bool { return len(r.appliedList()) == 1 })
+			eventually(t, "not running after the answer", func() bool {
+				h := r.haves.last("nats")
+				return h.Phase == string(delivery.PhaseRunning) && h.Composed == dig
+			})
+			r.mu.Lock()
+			n := len(r.backups)
+			r.mu.Unlock()
+			if n != row.backups {
+				t.Errorf("%d backups for the answer, want %d", n, row.backups)
+			}
+		})
+	}
 }
 
 // A data module with a good backup is applied after it; one that never ran has nothing to back up.

internal/agent/composition.go

@@ -21,6 +21,13 @@ type CompositionView struct {
 	Module string `json:"module"`
 	// Want is the module's want as the agent holds it; nil when it holds none.
 	Want *WantView `json:"want,omitempty"`
+	// Attempt is what the agent holds of the module's last apply that did not end running: failed, failed for a moment
+	// of the bus, or held for a failed backup, with the want's revision that hold took as answered; nil when none
+	// (issue 1585: why a module stays held was not readable from the engine).
+	Attempt *AttemptView `json:"attempt,omitempty"`
+	// Started says the agent has looked at the module once since it started (its baseline): what runs was taken from its
+	// have, or it was applied.
+	Started bool `json:"started"`
 	// Have is what the module's have key says now; nil when it says nothing.
 	Have *HaveView `json:"have,omitempty"`
 	// Composition is the revision read: the latest of the want's build, or the one whose digest was asked.
@@ -55,12 +62,34 @@ type OwnView struct {
 	Held map[string]string `json:"held,omitempty"`
 }
 
-// WantView is a want as the verb shows it.
+// WantView is a want as the verb shows it: as the agent holds it, at the revision of its key it last read.
 type WantView struct {
 	FP       string `json:"fp"`
 	Composed string `json:"composed,omitempty"`
 	Yes      string `json:"yes,omitempty"`
 	Removed  bool   `json:"removed,omitempty"`
+	// Self says the node composes the module itself (ADR 0547 §2).
+	Self bool `json:"self,omitempty"`
+	// Backup is the operator's answer to a failed backup the want carries, again or skip (ADR 0539 §16); empty when none.
+	Backup string `json:"backup,omitempty"`
+	Words  string `json:"words,omitempty"`
+	// Revision is the revision of the want's key the agent read last.
+	Revision uint64 `json:"revision"`
+}
+
+// AttemptView is the agent's record of a module's last apply that did not end running.
+type AttemptView struct {
+	FP       string `json:"fp"`
+	Composed string `json:"composed,omitempty"`
+	// WantRevision and CompositionRevision are the revisions of the want's and the composition's keys it was tried at.
+	WantRevision        uint64 `json:"want_revision"`
+	CompositionRevision uint64 `json:"composition_revision"`
+	// Held says it is held for a failed backup; AnsweredRevision is the want's revision the hold took, so an answer
+	// (again or skip) in a later revision releases it.
+	Held             bool   `json:"held,omitempty"`
+	AnsweredRevision uint64 `json:"answered_revision,omitempty"`
+	// Transient says it failed because the bus did not answer for a moment, and is tried again by the clock.
+	Transient bool `json:"transient,omitempty"`
 }
 
 // HaveView is a have as the verb shows it.
@@ -127,10 +156,16 @@ func (a *Agent) Composition(ctx context.Context, module, digest string, diff boo
 	fp := ""
 	if w.Present {
 		fp = w.Want.FP
-		view.Want = &WantView{FP: w.Want.FP, Composed: w.Want.Composed, Yes: w.Want.Yes, Removed: w.Want.Removed}
+		view.Want = &WantView{FP: w.Want.FP, Composed: w.Want.Composed, Yes: w.Want.Yes, Removed: w.Want.Removed,
+			Self: w.Want.Self, Backup: w.Want.Backup, Words: w.Want.Words, Revision: w.Rev}
 	} else if have != nil {
 		fp = have.FP
 	}
+	if att, ok := a.attempt[module]; ok {
+		view.Attempt = &AttemptView{FP: att.FP, Composed: att.Composed, WantRevision: att.WantRev, CompositionRevision: att.CompRev,
+			Held: att.Held, AnsweredRevision: att.AnsweredRev, Transient: att.Transient}
+	}
+	view.Started = a.checked[module]
 	latest, haveLatest := a.compositionFor(module, fp)
 	r, ran := a.running[module]
 	from, known := a.runningComposition(module)