diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 9f63130..baedc59 100644 --- a/internal/agent/agent.go +++ b/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 != "" { diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go index e2fed75..f5703a6 100644 --- a/internal/agent/agent_test.go +++ b/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. diff --git a/internal/agent/composition.go b/internal/agent/composition.go index 4035b8b..11d00e5 100644 --- a/internal/agent/composition.go +++ b/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)