an unrecognized act says so and lists nothing (V-556)

Reciting the allowlist answered a question he did not ask. She says the command
is not one she knows, once, and parks nothing.

cmd/mavend/clarify_test.go

@@ -231,10 +231,10 @@ func TestClarifyRestatedAnswerWins(t *testing.T) {
 	}
 }
 
-// TestClarifiedActOffAllowlistIsStillRefused — clarification fills in an
-// argument, it never grants authority. One tool is enabled so there is a
-// question to park at all (Vikunja #556); the answer names something else.
-func TestClarifiedActOffAllowlistIsStillRefused(t *testing.T) {
+// TestActOffAllowlistIsStillRefused — naming a capability is not being granted
+// one. Since Vikunja #556 an unresolved act no longer parks a question, so this
+// goes through applyAction, which is the only way an act runs.
+func TestActOffAllowlistIsStillRefused(t *testing.T) {
 	ctx := context.Background()
 	h, st, _ := newClarifyHandler(t)
 	marker := filepath.Join(t.TempDir(), "not-allowed-ran")
@@ -242,13 +242,11 @@ func TestClarifiedActOffAllowlistIsStillRefused(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if _, asked := h.askClarify(ctx, clarifyDec(router.IntentAct, router.Slots{Text: "сделай это"}, "сделай это")); !asked {
-		t.Fatal("an act with no fn should be asked about")
-	}
-	reply, handled := h.resolveClarifyAnswer(ctx, "rm "+marker)
-	if !handled {
-		t.Fatal("the answer should be consumed")
-	}
+	reply := h.applyAction(ctx, router.Decision{
+		Utterance: "rm " + marker,
+		Intent:    router.IntentAct,
+		Slots:     router.Slots{Fn: "rm " + marker, HasFn: true},
+	})
 	if strings.Contains(reply, "готово") {
 		t.Fatalf("an act that is not on the allowlist must not report success: %q", reply)
 	}
@@ -256,13 +254,12 @@ func TestClarifiedActOffAllowlistIsStillRefused(t *testing.T) {
 		t.Fatalf("a clarified act off the allowlist ran anyway: %v", err)
 	}
 	if tools, err := st.ListTools(ctx, "enabled"); err != nil || len(tools) != 1 {
-		t.Fatalf("clarify must not enable a tool: tools=%+v err=%v", tools, err)
+		t.Fatalf("an act must not enable a tool: tools=%+v err=%v", tools, err)
 	}
 }
 
-// TestClarifiedDestructiveActStillNeedsConfirm — the confirm gate survives the
-// clarify path.
-func TestClarifiedDestructiveActStillNeedsConfirm(t *testing.T) {
+// TestDestructiveActStillNeedsConfirm — the confirm gate stands on the act path.
+func TestDestructiveActStillNeedsConfirm(t *testing.T) {
 	ctx := context.Background()
 	h, st, _ := newClarifyHandler(t)
 	marker := filepath.Join(t.TempDir(), "destructive-ran")
@@ -270,18 +267,16 @@ func TestClarifiedDestructiveActStillNeedsConfirm(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if _, asked := h.askClarify(ctx, clarifyDec(router.IntentAct, router.Slots{Text: "сделай это"}, "сделай это")); !asked {
-		t.Fatal("expected a question")
-	}
-	reply, handled := h.resolveClarifyAnswer(ctx, "delete_backups")
-	if !handled {
-		t.Fatal("the answer should be consumed")
-	}
+	reply := h.applyAction(ctx, router.Decision{
+		Utterance: "delete_backups",
+		Intent:    router.IntentAct,
+		Slots:     router.Slots{Fn: "delete_backups", HasFn: true},
+	})
 	if !strings.Contains(reply, "да") || h.pending == nil {
-		t.Fatalf("a clarified destructive act must still park a confirm: reply=%q pending=%+v", reply, h.pending)
+		t.Fatalf("a destructive act must park a confirm: reply=%q pending=%+v", reply, h.pending)
 	}
 	if _, err := os.Stat(marker); !os.IsNotExist(err) {
-		t.Fatalf("a clarified destructive act ran before confirmation: %v", err)
+		t.Fatalf("a destructive act ran before confirmation: %v", err)
 	}
 }
 
@@ -575,14 +570,12 @@ func TestARestartExpiresTheParkedQuestion(t *testing.T) {
 // line. None of them was ever an answer.
 func TestClarifyStepsAsideForItsOwnRequest(t *testing.T) {
 	ctx := context.Background()
-	h, st, _ := newClarifyHandler(t)
-	// One enabled tool, so there is a question to park (Vikunja #556).
-	if err := st.EnableTool(ctx, "uptime", []string{"true"}, false, "test", h.now()); err != nil {
-		t.Fatal(err)
-	}
+	h, _, _ := newClarifyHandler(t)
 
-	if _, asked := h.askClarify(ctx, clarifyDec(router.IntentAct, router.Slots{Text: "выключи свет в спальне"}, "выключи свет в спальне")); !asked {
-		t.Fatal("an act with no fn should be asked about")
+	// A reminder, not the act this bug was found on: since Vikunja #556 an act
+	// no longer parks anything, so it can no longer eat the turn after it.
+	if _, asked := h.askClarify(ctx, clarifyDec(router.IntentReminder, router.Slots{Text: "напомни позвонить маме"}, "напомни позвонить маме")); !asked {
+		t.Fatal("a reminder with no time should be asked about")
 	}
 	if reply, handled := h.resolveClarifyAnswer(ctx, "кто изобрёл телефон"); handled {
 		t.Fatalf("a world question must route as itself, got %q", reply)
@@ -629,62 +622,27 @@ func TestClarifyQuestionShapedAnswerThatFillsTheGapStillLands(t *testing.T) {
 	}
 }
 
-// TestActWithNothingOnTheOtherEndNamesTheGap — Vikunja #556. With no tool
-// enabled, "Что сделать?" has no answer he could give, so she says so and parks
-// nothing rather than spending three turns on a request she cannot fulfil.
-func TestActWithNothingOnTheOtherEndNamesTheGap(t *testing.T) {
+// TestUnresolvedActSaysItDoesNotKnowTheCommand — Vikunja #556. "Что сделать?"
+// has no answer he can give, so an act that matched no capability is refused in
+// one line and nothing is parked. It does not recite what she can do instead.
+func TestUnresolvedActSaysItDoesNotKnowTheCommand(t *testing.T) {
 	ctx := context.Background()
-	h, _, _ := newClarifyHandler(t)
+	h, st, _ := newClarifyHandler(t)
+	// Enabled tools change nothing here: this act matched none of them.
+	if err := st.EnableTool(ctx, "uptime", []string{"true"}, false, "test", h.now()); err != nil {
+		t.Fatal(err)
+	}
 
 	reply, spoken := h.askClarify(ctx, clarifyDec(router.IntentAct, router.Slots{Text: "выключи свет"}, "выключи свет"))
-	if !spoken || reply == "" {
-		t.Fatal("an act with nothing on the other end must still say something")
+	if !spoken || reply != actNotRecognized {
+		t.Fatalf("reply = %q spoken=%v, want %q", reply, spoken, actNotRecognized)
 	}
-	if strings.Contains(reply, "?") {
-		t.Errorf("reply = %q, want a statement, not a question", reply)
+	if strings.Contains(reply, "uptime") {
+		t.Errorf("reply = %q, want no list of capabilities he did not ask about", reply)
 	}
 	if h.clarifyStore.Get(voiceDialogueID, h.now()) != nil {
 		t.Error("nothing to ask about, so nothing may be parked")
 	}
 }
 
-// TestActClarifyNamesWhatSheCanDo — the other half. With tools enabled the
-// question stands, and it names them so it is answerable.
-func TestActClarifyNamesWhatSheCanDo(t *testing.T) {
-	ctx := context.Background()
-	h, st, _ := newClarifyHandler(t)
-	for _, name := range []string{"uptime", "disk"} {
-		if err := st.EnableTool(ctx, name, []string{"true"}, false, "test", h.now()); err != nil {
-			t.Fatal(err)
-		}
-	}
 
-	reply, asked := h.askClarify(ctx, clarifyDec(router.IntentAct, router.Slots{Text: "сделай это"}, "сделай это"))
-	if !asked {
-		t.Fatal("with tools enabled she should still ask")
-	}
-	for _, want := range []string{"uptime", "disk"} {
-		if !strings.Contains(reply, want) {
-			t.Errorf("reply = %q, want it to name %q", reply, want)
-		}
-	}
-	if h.clarifyStore.Get(voiceDialogueID, h.now()) == nil {
-		t.Error("the request must be parked so the answer can land")
-	}
-}
-
-// TestFnClarifyCapsTheListItRecites — a spoken sentence cannot carry twelve
-// names, which is how many this deployment enables.
-func TestFnClarifyCapsTheListItRecites(t *testing.T) {
-	allow := []string{"a", "b", "c", "d", "e", "f", "g", "h"}
-	reply, ask := fnClarify(allow, 1)
-	if !ask {
-		t.Fatal("a non-empty allowlist is still a question")
-	}
-	if strings.Contains(reply, "g") || strings.Contains(reply, "h") {
-		t.Errorf("reply = %q, want the list capped at %d", reply, namedActsCap)
-	}
-	if !strings.Contains(reply, "…") {
-		t.Errorf("reply = %q, want it to admit the list was cut", reply)
-	}
-}