✨ feat(review): route blocking verdicts to a human

changelog.d/added-reviewer-routes-to-humans.md

@@ -0,0 +1 @@
+- Review comments now route: a `requires_human` or `reject` verdict opens with a pinned `**HUMAN DECISION NEEDED**` marker maintainers can filter on, mentions the PR author when that author is a person (never an app/bot account, which notifies nobody), and asks the reviewer to state plainly what it could not judge instead of approving around it.

src/pkg/review/prompts.go

@@ -86,9 +86,48 @@ func buildPublishInstruction(pr PullRequest) string {
 	b.WriteString("Say nothing rather than pad. Do NOT post a comment that is only nits, only praise, or a restatement of the diff. If this perspective found nothing a human needs, skip the comment entirely and just return the JSON.\n")
 	b.WriteString("If the PR body claims behavior the diff does not implement, say so with file:line — that gap is one of the most useful things you can report.\n")
 	b.WriteString("Be brief and specific. One comment, at most a few findings, worst first.\n")
+	b.WriteString(buildRoutingInstruction(pr))
 	return b.String()
 }
 
+// buildRoutingInstruction is what turns a verdict into a decision.
+//
+// A hive that cannot merge produces reviews whose only possible outcome is a
+// human acting on them. But a correct, well-cited comment buried in a queue of
+// hundreds is not actionable: nothing distinguishes "a human must decide this"
+// from routine review noise. Routing is therefore not a nicety on top of the
+// review — it is the step that makes the review reach anyone.
+func buildRoutingInstruction(pr PullRequest) string {
+	var b strings.Builder
+	b.WriteString("\nROUTING — make a needed human decision findable.\n")
+	b.WriteString("If your verdict is requires_human or reject, the FIRST line of the comment must be exactly:\n")
+	b.WriteString("  **HUMAN DECISION NEEDED** — <one line naming the decision only a human can make>\n")
+	b.WriteString("A maintainer triaging a long queue filters on that marker; without it a blocking finding reads as one more comment and is skipped.\n")
+	if handle := mentionableAuthor(pr.Author); handle != "" {
+		fmt.Fprintf(&b, "On that same line, mention @%s (the PR author) so the person who can act is notified.\n", handle)
+	} else {
+		b.WriteString("Do NOT @-mention the PR author: this PR was opened by an app or bot account, so a mention notifies nobody. The marker line is the routing.\n")
+	}
+	b.WriteString("Report the limits of your own review. If you could not judge part of this PR — missing context, an unfamiliar subsystem, an ambiguous requirement, a change you cannot test — say so plainly and use requires_human. Naming what you could not verify is more useful than a confident guess, and omitting it is how an unreviewed change gets waved through on the strength of an automated approval.\n")
+	return b.String()
+}
+
+// mentionableAuthor returns the bare @-handle for a PR author when mentioning
+// it would reach a person, and "" when it would not.
+//
+// On a hive fleet most PRs are agent-authored, so the author is an App
+// ("app/<name>") or a bot ("<name>[bot]"). @-mentioning either notifies no one
+// — it renders as a link and nothing else — while still looking to a reader
+// like the review was routed somewhere. That false signal is worse than no
+// mention at all, because it suggests a human is already on it.
+func mentionableAuthor(author string) string {
+	a := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(author), "@"))
+	if a == "" || strings.HasSuffix(a, "[bot]") || strings.Contains(a, "/") {
+		return ""
+	}
+	return a
+}
+
 func BuildPerspectivePrompts(pr PullRequest, perspectives []Perspective) map[Perspective]string {
 	if len(perspectives) == 0 {
 		perspectives = DefaultPerspectives

src/pkg/review/publish_prompt_test.go

@@ -111,3 +111,81 @@ func TestPromptPublishAppliesToEveryPerspective(t *testing.T) {
 		}
 	}
 }
+
+// TestRoutingMarkerIsPinned pins the exact marker a maintainer filters on. The
+// value of the marker is entirely in its being identical across every repo and
+// every reviewer; a reworded variant is unsearchable and therefore useless.
+func TestRoutingMarkerIsPinned(t *testing.T) {
+	got := BuildPerspectivePromptOpts(PerspectiveCorrectness, testPR(), true)
+
+	if !strings.Contains(got, "**HUMAN DECISION NEEDED**") {
+		t.Error("publish prompt does not pin the HUMAN DECISION NEEDED marker")
+	}
+	if !strings.Contains(got, "requires_human or reject") {
+		t.Error("publish prompt does not tie the marker to the blocking verdicts")
+	}
+}
+
+// TestRoutingDoesNotMentionBotAuthors is the point of the author check: the
+// common case on a hive is an agent-authored PR, and mentioning the App that
+// opened it notifies nobody while looking like the review was routed.
+func TestRoutingDoesNotMentionBotAuthors(t *testing.T) {
+	for _, author := range []string{
+		"kubestellar-hive[bot]",
+		"app/kubestellar-hive",
+		"",
+	} {
+		pr := testPR()
+		pr.Author = author
+		got := BuildPerspectivePromptOpts(PerspectiveCorrectness, pr, true)
+
+		if strings.Contains(got, "(the PR author) so the person who can act") {
+			t.Errorf("author %q: prompt asks the reviewer to mention a non-human author", author)
+		}
+		if !strings.Contains(got, "Do NOT @-mention the PR author") {
+			t.Errorf("author %q: prompt omits the do-not-mention instruction", author)
+		}
+	}
+}
+
+// TestRoutingMentionsHumanAuthors is the converse: when a person opened the PR,
+// the mention is the fastest path from finding to decision.
+func TestRoutingMentionsHumanAuthors(t *testing.T) {
+	pr := testPR()
+	pr.Author = "@clubanderson"
+	got := BuildPerspectivePromptOpts(PerspectiveCorrectness, pr, true)
+
+	if !strings.Contains(got, "mention @clubanderson (the PR author)") {
+		t.Error("prompt does not ask the reviewer to mention the human PR author")
+	}
+	// The leading @ must not be doubled when the author already carries one.
+	if strings.Contains(got, "@@") {
+		t.Error("prompt double-prefixed the author handle with @")
+	}
+	if strings.Contains(got, "Do NOT @-mention the PR author") {
+		t.Error("prompt suppressed the mention for a human author")
+	}
+}
+
+// TestRoutingAsksForReviewLimits covers the capability-honesty half: a reviewer
+// that cannot judge something must say so rather than approve around it.
+func TestRoutingAsksForReviewLimits(t *testing.T) {
+	got := BuildPerspectivePromptOpts(PerspectiveCorrectness, testPR(), true)
+
+	if !strings.Contains(got, "Report the limits of your own review") {
+		t.Error("publish prompt does not ask the reviewer to report what it could not judge")
+	}
+}
+
+// TestRoutingIsOptInWithPublish keeps routing on the same switch as publishing:
+// a hive that has not opted into comments must not have its prompt changed.
+func TestRoutingIsOptInWithPublish(t *testing.T) {
+	got := BuildPerspectivePrompt(PerspectiveCorrectness, testPR())
+
+	if strings.Contains(got, "HUMAN DECISION NEEDED") {
+		t.Error("default prompt contains the routing marker; routing must be opt-in with publishing")
+	}
+	if strings.Contains(got, "ROUTING") {
+		t.Error("default prompt contains the routing block; routing must be opt-in with publishing")
+	}
+}