Tell a refusal as one and an unanswered call as not known; ask a plan that fails again within the hour (issues 1316, 1314, review round 1)

cmd/mesh-controller/asker.go

@@ -409,7 +409,9 @@ func (a *asker) reconcile(ctx context.Context) error {
 	for _, r := range all {
 		k := partKey(r.Condition, r.Part)
 		if r.State == string(asks.OutcomeChosen) && now.Sub(r.Ended) < askAgainAfterAnswer {
-			answered[k] = r
+			if prior, has := answered[k]; !has || r.Ended.After(prior.Ended) {
+				answered[k] = r
+			}
 		}
 		if r.State == string(asks.OutcomeRefused) {
 			if prior, has := refused[k]; !has || r.Opened.After(prior.Opened) {
@@ -500,7 +502,9 @@ func (a *asker) reconcile(ctx context.Context) error {
 					continue // refused lately, and nothing it was refused for has changed: asked again after the wait
 				}
 			}
-			if r, done := answered[key]; done && sameAsked(r.Actions, p.actions) {
+			// **A condition raised after the answered ask ended is a new one** (novox/hq ADR 0449 §2): a retried
+			// plan that fails again is asked again, not held back for the hour its earlier answer holds.
+			if r, done := answered[key]; done && sameAsked(r.Actions, p.actions) && !c.Raised.After(r.Ended) {
 				if _, held := byCondition[key]; !held {
 					continue
 				}
@@ -924,33 +928,53 @@ func (a *asker) Decided(ctx context.Context, body []byte) error {
 	return nil
 }
 
-// tellRefused tells the operator that the answer they gave to a plan's ask was refused (novox/hq ADR 0449 §3): the
+// tellRefused tells the operator what became of the answer they gave to a plan's ask, when it was not done (novox/hq
-// controller's `plans` verb refuses a Retry, Start or Stop when the plan moved on between the ask and the answer,
+// ADR 0449 §3): the controller's `plans` verb refuses a Retry, Start or Stop when the plan moved on between the ask and
-// and the refusal was only recorded, so the operator who pressed the answer heard nothing. One plain notice with the
+// the answer, and the refusal was only recorded, so the operator who pressed the answer heard nothing. **A refusal is
-// ask's headline and the refusal's words, beside the hand act the caller still records. An answer to any other ask
+// told as one, and nothing else is**: a verb that said no changed nothing, but a call that got no answer (a timeout, a
-// is not told here. A notice that cannot be told is said in the log and changes nothing.
+// refused grant) may have run, and is told as not known. One plain notice with the ask's headline, beside the hand act
-func (a *asker) tellRefused(ctx context.Context, r asked, act conditions.Action, refusal error) {
+// the caller still records. An answer to any other ask is not told here. A notice that cannot be told is said in the
+// log and changes nothing.
+func (a *asker) tellRefused(ctx context.Context, r asked, act conditions.Action, failure error) {
 	if a.tell == nil || act.Verb != broker.ControllerSeat+".plans" {
 		return
 	}
-	lines := []string{
+	var lines []string
-		fmt.Sprintf("Your answer, %s, could not be done: %s.", act.Label, strings.TrimRight(refusalWords(refusal), ".")),
+	var refused verbRefused
-		"Nothing was changed.",
+	if errors.As(failure, &refused) {
+		lines = []string{
+			fmt.Sprintf("Your answer, %s, could not be done: %s.", act.Label, strings.TrimRight(refusalWords(refused.words, act), ".")),
+			"Nothing was changed.",
+		}
+	} else {
+		lines = []string{
+			fmt.Sprintf("Your answer, %s, was given, and it is not known whether it was done.", act.Label),
+			"Look at the delivery before you answer again.",
+		}
 	}
 	if err := a.tell(ctx, r.Ask.Headline, lines); err != nil {
 		a.logf("the operator could NOT be told that %s was refused (ask %s): %v", act.Label, r.ID, err)
 	}
 }
 
-// refusalWords is a refused verb's words as the operator reads them: without the verb's address in front, with the
+// refusalWords is a verb's refusal as the operator reads it: the plans named as "the plan" (plainRefusal), and, where
-// plans named as "the plan" (plainRefusal), and, where they still carry an identifier the operator's channel
+// the words still carry an identifier the operator's channel withholds, one sentence of the answer's own.
-// withholds, only that the controller refused it.
+func refusalWords(words string, act conditions.Action) string {
-func refusalWords(err error) string {
+	return plainRefusalOr(words, refusalFallback(act))
-	text := err.Error()
+}
-	if i := strings.Index(text, " refused: "); i >= 0 {
+
-		text = text[i+len(" refused: "):]
+// refusalFallback is what an answer's refusal says when its own words cannot be said: one sentence for each answer
+// to a plan's ask, never another answer's.
+func refusalFallback(act conditions.Action) string {
+	switch {
+	case act.Arguments["retry"] != "":
+		return "a retry would not run now; the details say why"
+	case act.Arguments["go"] != "":
+		return "the delivery could not be started now; the details say why"
+	case act.Arguments["stop"] != "":
+		return "the delivery could not be stopped now; the details say why"
 	}
-	return plainRefusal(text)
+	return "the controller refused it; the details say why"
 }
 
 // handActWarrant is the verb an act the operator chose on a warrant is recorded under: a person's decision,

cmd/mesh-controller/asker_test.go

@@ -1105,7 +1105,7 @@ func TestARefusedAnswerToAPlansAskIsToldOnceAndStillRecorded(t *testing.T) {
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			r := newAskerRig(t)
-			r.refuses = errors.New("mesh-controller.plans refused: " + plan + " is done; there is nothing to retry")
+			r.refuses = verbRefused{verb: "mesh-controller.plans", words: plan + " is done; there is nothing to retry"}
 			r.open = []conditions.Condition{tc.c}
 			_ = r.a.reconcile(context.Background())
 			w := r.warrantFor(t, tc.c.Key, tc.label)
@@ -1147,7 +1147,7 @@ func TestADoneAnswerTellsNothingAndOnlyAPlansRefusalIsTold(t *testing.T) {
 	}
 
 	r = newAskerRig(t)
-	r.refuses = errors.New("node-service-manager.restart refused: unit not found")
+	r.refuses = verbRefused{verb: "node-service-manager.restart", words: "unit not found"}
 	module := conditions.Condition{Key: "module.openrazer.g14.unhealthy", Severity: conditions.Warning,
 		Headline: "openrazer not working on g14", Needs: "restart its service openrazer-daemon on g14.",
 		Explanation: "Needs you: restart it.", Actions: []conditions.Action{{Label: "Restart",
@@ -1160,3 +1160,81 @@ func TestADoneAnswerTellsNothingAndOnlyAPlansRefusalIsTold(t *testing.T) {
 		t.Fatalf("a refused restart told %+v, acts %+v", r.told, r.acts)
 	}
 }
+
+// 1316, review round 1: a call that got no answer may have run, so it is told as not known, never as "nothing was
+// changed"; and the sentence a refusal falls back to fits the answer given.
+func TestAnAnswerThatGotNoAnswerIsToldAsNotKnownNotAsRefused(t *testing.T) {
+	plan := "plan-1792433168676870614"
+	r := newAskerRig(t)
+	r.refuses = errors.New("mesh-controller did not answer plans within 30s")
+	c := failedPlanCondition(plan)
+	r.open = []conditions.Condition{c}
+	_ = r.a.reconcile(context.Background())
+	answerWith(t, r, r.warrantFor(t, c.Key, "Retry"))
+	if len(r.told) != 1 {
+		t.Fatalf("told %+v", r.told)
+	}
+	said := strings.Join(r.told[0].lines, " ")
+	if !strings.Contains(said, "not known whether") || strings.Contains(said, "Nothing was changed") {
+		t.Errorf("a call that may have run is told as %q", said)
+	}
+	if why, ok := conditions.Plain(said); !ok {
+		t.Errorf("not plain: %s: %q", why, said)
+	}
+}
+
+func TestEachAnswersRefusalFallsBackToItsOwnSentence(t *testing.T) {
+	plan := "plan-1792433168676870614"
+	for _, tc := range []struct {
+		act  conditions.Action
+		want string
+	}{
+		{retryAction(plan), "a retry would not run now"},
+		{conditions.Action{Label: "Start", Verb: "mesh-controller.plans", Arguments: map[string]string{"go": plan}}, "could not be started now"},
+		{conditions.Action{Label: "Stop", Verb: "mesh-controller.plans", Arguments: map[string]string{"stop": plan}}, "could not be stopped now"},
+	} {
+		// Words the operator's channel withholds: a hash and a dotted name.
+		got := refusalWords("build 48bda475dunst at mesh.seat.x refused", tc.act)
+		if !strings.Contains(got, tc.want) {
+			t.Errorf("%s: %q, want %q", tc.act.Label, got, tc.want)
+		}
+		if _, ok := conditions.Plain(got); !ok {
+			t.Errorf("%s: the fallback is not plain: %q", tc.act.Label, got)
+		}
+	}
+}
+
+// 1314/AC6, ADR 0449 §2: a retried plan that fails again within the hour of the answer is asked again, because the
+// condition that comes back is a new one, raised after the answered ask ended; the same condition still open is not.
+func TestARetriedPlanThatFailsAgainWithinTheHourIsAskedAgain(t *testing.T) {
+	r := newAskerRig(t)
+	c := failedPlanCondition("plan-1792433168676870614")
+	c.Raised = r.now.Add(-time.Minute)
+	r.open = []conditions.Condition{c}
+	_ = r.a.reconcile(context.Background())
+	answerWith(t, r, r.warrantFor(t, c.Key, "Retry"))
+	// Still open right after the answer: not asked again at once.
+	r.now = r.now.Add(time.Minute)
+	_ = r.a.reconcile(context.Background())
+	if n := len(r.asksSent(t)); n != 1 {
+		t.Fatalf("asked again while the same condition stayed open: %d", n)
+	}
+	// The retry ran, the plan left failed and the condition ended.
+	r.now = r.now.Add(time.Minute)
+	r.open = nil
+	_ = r.a.reconcile(context.Background())
+	// It fails again, ten minutes after the answer: a condition raised after the answered ask ended.
+	r.now = r.now.Add(8 * time.Minute)
+	again := failedPlanCondition("plan-1792433168676870614")
+	again.Raised = r.now
+	r.open = []conditions.Condition{again}
+	if err := r.a.reconcile(context.Background()); err != nil {
+		t.Fatal(err)
+	}
+	if n := len(r.asksSent(t)); n != 2 {
+		t.Fatalf("the second failure was asked %d times in all, want 2", n)
+	}
+	if len(r.called) != 1 {
+		t.Fatalf("something retried without an answer: %v", r.called)
+	}
+}

cmd/mesh-controller/asker_wire.go

@@ -135,6 +135,12 @@ func (b busAsked) All(ctx context.Context) ([]asked, error) {
 	return out, nil
 }
 
+// verbRefused is a verb's own refusal of an act: the seat answered, and said no. Any other error of callAction (a bus
+// that did not answer, a grant that is refused) is not one: the verb may have run.
+type verbRefused struct{ verb, words string }
+
+func (e verbRefused) Error() string { return e.verb + " refused: " + e.words }
+
 // callAction performs an action's verb as the controller, through the grant that names it.
 func callAction(conn *nats.Conn) func(ctx context.Context, a conditions.Action, args map[string]string) error {
 	return func(ctx context.Context, a conditions.Action, args map[string]string) error {
@@ -168,7 +174,7 @@ func callAction(conn *nats.Conn) func(ctx context.Context, a conditions.Action,
 			return err
 		}
 		if answer.Error != "" {
-			return fmt.Errorf("%s refused: %s", a.Verb, answer.Error)
+			return verbRefused{verb: a.Verb, words: answer.Error}
 		}
 		return nil
 	}

cmd/mesh-controller/plan_failed.go

@@ -92,12 +92,17 @@ var (
 // delivery. Whatever still breaks the plain rule (a hash, a dotted name) is left to the condition's evidence, and the
 // words say only that a retry would not run.
 func plainRefusal(refusal string) string {
+	return plainRefusalOr(refusal, "a retry would not run now; the details say why")
+}
+
+// plainRefusalOr is plainRefusal with the sentence to say when the refusal's own words cannot be said plainly.
+func plainRefusalOr(refusal, fallback string) string {
 	r := planIDWord.ReplaceAllString(refusal, "the plan")
 	r = walkWd.ReplaceAllString(r, "deliveries")
 	r = walkOne.ReplaceAllString(r, "delivery")
 	r = strings.TrimSpace(r)
 	if _, ok := conditions.Plain(r); !ok {
-		return "a retry would not run now; the details say why"
+		return fallback
 	}
 	return r
 }