A composer's mark never covers another cause, and a run that ended marks and clears nothing (issue 1126, review of the switch, round 1)

NotComposed marked from a read have whatever its phase: an apply that failed,
naming the want's build and digest, became a composer's mark, which baseline
then took for what runs, and never applied again. It now marks only a have that
runs, says nothing, or is a composer's mark already. And its loop, whose reads
can take seconds, checks the run's context at each module, so a run that ended
writes no have.

internal/agent/agent.go

@@ -318,6 +318,11 @@ type notComposedMark struct {
 // name, and only the node's composition of it failed.
 const NotComposedDetail = "the node cannot compose it, and keeps running what it ran: "
 
+// composerMark says h is a mark NotComposed wrote: failed, its detail opening with NotComposedDetail.
+func composerMark(h Have) bool {
+	return h.Phase == string(delivery.PhaseFailed) && strings.HasPrefix(h.Detail, NotComposedDetail)
+}
+
 // HavesReadErr is what a Haves may answer besides Read: a have absent (false, nil) told from one that could not be read
 // (false, the error).
 type HavesReadErr interface {
@@ -375,6 +380,9 @@ func (a *Agent) NotComposed(ctx context.Context, why string) {
 	a.mu.Unlock()
 	sort.Strings(modules)
 	for _, module := range modules {
+		if ctx.Err() != nil {
+			return // the run ended: what it would mark or clear is the next run's to say (issue 1126)
+		}
 		a.mu.Lock()
 		mark, marked := a.notComposed[module]
 		r, ran := a.running[module]
@@ -405,6 +413,9 @@ func (a *Agent) NotComposed(ctx context.Context, why string) {
 			} else if h, found, unread := a.readHave(ctx, module); found {
 				mark.prior = h // what its last have said runs: this agent has run nothing of it since its start
 			} else if unread {
+				if ctx.Err() != nil {
+					return
+				}
 				// What its have names cannot be read (the bus paused, issue 1126): a mark written now would drop the build
 				// and digest it names. Nothing is written; the next refusal marks it once the have reads.
 				a.mu.Lock()
@@ -413,6 +424,19 @@ func (a *Agent) NotComposed(ctx context.Context, why string) {
 				continue
 			}
 		}
+		// A have that already says another cause (an apply that failed, a backup held, an apply under way) keeps it: a
+		// composer's mark over it would hide that cause, and baseline would take the mark for what runs (review of the
+		// switch, round 1).
+		if p := mark.prior.Phase; !marked && p != "" && p != string(delivery.PhaseRunning) && !composerMark(mark.prior) {
+			a.mu.Lock()
+			a.sayOnce("unmarked-cause:"+module, fmt.Sprintf("%s is not marked: its have says %s (%s), and the node cannot compose it: %s",
+				module, p, mark.prior.Detail, why))
+			a.mu.Unlock()
+			continue
+		}
+		if ctx.Err() != nil {
+			return
+		}
 		mark.why = why
 		a.mu.Lock()
 		a.notComposed[module] = mark
@@ -948,7 +972,7 @@ func (a *Agent) baseline(ctx context.Context, module string, w wantNow, comp com
 				// A have a composer marked failed (NotComposed) names what still runs: taken as running, so a mark this
 				// agent did not make clears once what it names is what the want names (issue 1126, the stale marks after
 				// the seed's revert).
-				marked := h.Phase == string(delivery.PhaseFailed) && strings.HasPrefix(h.Detail, NotComposedDetail)
+				marked := composerMark(h)
 				if marked {
 					h.Detail = ""
 				}

internal/agent/agent_test.go

@@ -342,6 +342,21 @@ func TestStartingOnWhatTheKeptDeclarationHoldsAppliesNothing(t *testing.T) {
 // What the node-engine itself last applied is what runs after a restart, not the kept declaration.
 func TestStartingOnItsOwnHaveAppliesNothing(t *testing.T) {
 	t.Run("says the have again", testStartingOnItsOwnHaveSaysTheHaveAgainSoItsHealthIsThisEngines)
+	// An apply that failed is not what runs, though its have names the want's build and digest: the agent starting on
+	// it applies again (review of the switch, round 1).
+	t.Run("applies again after an apply failure", func(t *testing.T) {
+		b := newBed(t)
+		fp := fpOf("a")
+		ck, craw, dig := composition(t, "avahi", fp, false, file("avahi", "x", "1"))
+		wk, wraw := wantFor(t, "avahi", fp, dig, "")
+		b.haves.stored = map[string]Have{"avahi": {Module: "avahi", FP: fp, Composed: dig, Phase: string(delivery.PhaseFailed),
+			Detail: "apply: unit failed to start"}}
+		b.comps.before(ck, craw)
+		b.wants.before(wk, wraw)
+		stop := b.run()
+		defer stop()
+		eventually(t, "a failed apply was not applied again", func() bool { return len(b.appliedList()) == 1 })
+	})
 	// Issue 1126, the stale marks after the seed's revert: a have a composer marked failed names what still runs; the
 	// agent starting on it applies nothing and says it running, so a mark it did not make clears.
 	t.Run("clears a composer's mark", func(t *testing.T) {
@@ -1084,6 +1099,11 @@ func TestTheNodesOwnCompositionIsAppliedWhenItDiffersAndARecordMoveWaitsForAYes(
 	b.haves.mu.Unlock()
 	want("unread", fpOf("d"), "")
 	want("fresh", fpOf("e"), "") // never ran, its have absent: marked, naming nothing it runs
+	// A have that already says another cause keeps it (review of the switch, round 1): an apply that failed.
+	brokenFP, brokenDig := fpOf("f"), "sha256:"+strings.Repeat("8", 64)
+	b.haves.Write(context.Background(), Have{Module: "broken", FP: brokenFP, Composed: brokenDig, Phase: string(delivery.PhaseFailed),
+		Detail: "apply: unit failed to start"})
+	want("broken", brokenFP, "")
 	eventually(t, "unread's want was not taken", func() bool {
 		b.ag.mu.Lock()
 		defer b.ag.mu.Unlock()
@@ -1096,6 +1116,17 @@ func TestTheNodesOwnCompositionIsAppliedWhenItDiffersAndARecordMoveWaitsForAYes(
 	if h := b.haves.last("fresh"); h.Phase != string(delivery.PhaseFailed) || h.FP != "" {
 		t.Errorf("a module that never ran, its have absent, is not marked: %+v", h)
 	}
+	if h := b.haves.last("broken"); h.Detail != "apply: unit failed to start" || h.Composed != brokenDig {
+		t.Errorf("an apply failure was overwritten by a composer's mark: %+v", h)
+	}
+	// A run that ended marks nothing: what it would say is the next run's (review of the switch, round 1).
+	ended, end := context.WithCancel(context.Background())
+	end()
+	before := len(b.haves.log)
+	b.ag.NotComposed(ended, "a refusal from a run that ended")
+	if len(b.haves.log) != before {
+		t.Errorf("a run that ended wrote %d haves", len(b.haves.log)-before)
+	}
 	b.ag.NotComposed(context.Background(), "")
 	for _, row := range []struct{ module, composed string }{{"web", dig}, {"idle", idleDig}} {
 		if h := b.haves.last(row.module); h.Phase != string(delivery.PhaseRunning) || h.Composed != row.composed || len(h.Sealed) != 1 {