✨ feat(review): route blocking verdicts to a human - #7527
Merged
Merged
Conversation
A hive at ACMM L5 cannot merge, so the only possible outcome of a review
is a human acting on it. Reviewers already produce well-cited comments
with requires_human and reject verdicts, but nothing carried those to a
person: a blocking finding landed in a queue of hundreds looking exactly
like routine review noise, and the two reviews posted on the projectbluefin
spoke contained zero mentions and no way to filter for "a human must
decide this".
Add a routing block to the publish half of the kick. A requires_human or
reject verdict must open the comment with a pinned
`**HUMAN DECISION NEEDED**` marker, which is the string a maintainer
triaging a long queue can actually search on; its whole value is being
identical everywhere, so it is pinned by test.
Mention the PR author only when the author is a person. On a hive fleet
most PRs are agent-authored, so the author is an App ("app/<name>") or a
bot ("<name>[bot]"), and @-mentioning either notifies no one while still
reading as though the review had been routed somewhere. That false signal
is worse than no mention, so for those authors the prompt explicitly
suppresses the mention and leans on the marker instead.
Also ask the reviewer to report the limits of its own review. An honest
"I could not verify X" is more useful than a confident guess, and leaving
it out is how an unreviewed change gets waved through on the strength of
an automated approval.
Routing rides the existing review.post_comments switch, so a hive that
has not opted into commenting sees no prompt change.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Andrew Anderson <andy@clubanderson.com>
kubestellar-prow
Bot
added
dco-signoff: yes
Member
Author
|
/lgtm |
Member
Author
|
/approve |
Contributor
|
@clubanderson: you cannot LGTM your own PR. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: clubanderson The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
kubestellar-prow
Bot
added
the
approved
Contributor
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
This was referenced Sep 18, 2026
Closed
clubanderson
added a commit
that referenced
this pull request
Sep 18, 2026
Carries v4 e74d45e..201c69c (v4.54.2, v4.55.0, v4.55.1) into v5, including #7510 (cmd/hive domain extraction), #7532 (manager.go split), #7527/#7528 (review routing + breadth-first slots), #7523 (token-access audit backport of v5 #6303), #7515 (branch-protection facts on blocked PR pills), #7535, #7539, #7520. Resolution notes: - cmd/hive, pkg/agent: declaration-level 3-way merge against base ff7630e; v5 seams (spokeStatePath, SetHubPushedDashboardURL, applyModeUnscheduledAlert, mutation boundary, runDuplicateSweepIfDue) preserved. F24 tombstone intact. - pkg/github/client.go: v5 fetchPRs signature (RepoPRBreakdown) + v4 BaseRef/prBaseRef; added Client.requiredStatusCheckContexts shim so v4's protection_facts.go compiles against v5, where the sweep lives in pkg/github/automerge. - pkg/github/token_access_audit{,_test}.go: v4 side (its backport of v5's own #6303, now using testutil.Eventually). - pkg/github/automerge/f3_trusted_merger_source_test.go: v4's f3ReadPackage path adjusted one level deeper for v5's subpackage. - pkg/review/prompts.go: v4 routing instruction + mentionableAuthor added; v5's #6212 deletion of dead BuildPerspectivePrompts / BuildSequentialPrompt kept. - pkg/agent tests: v4's normalizeModelName cases rewritten against v5's normalizeModelNameForBackend (v5 removed the wrapper in #5795). - docs/knowledge-curator.md: v5 promotion-only body + v4's knowledge.git_sources section (code exists on v5). roadmap.md: v4 rows. api-reference.md: v5 table, citations re-resolved (--fix). - .github/workflows/v2-ci.yml: v5 UID-isolation step + v4's fuller audit-log step comment. Refs #7463 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Andy Anderson <andy@clubanderson.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A hive at ACMM L5 cannot merge, so the only possible outcome of a review is a human acting on it. The reviewer now posts well-cited comments (#7500) — but nothing carries a blocking verdict to a person.
Verified on the
projectbluefinspoke. Both reviews posted through the relay are substantive and correctly cited:Both contain zero @mentions, and neither offers any way for a maintainer to distinguish "a human must decide this" from routine review noise. In a queue of ~423 open PRs that difference is the whole ballgame: a correct finding nobody filters for is a finding nobody acts on.
Change
Adds a routing block to the publish half of the kick (
buildRoutingInstruction).Pinned marker. A
requires_humanorrejectverdict must open the comment with exactly**HUMAN DECISION NEEDED** — <one line>. Its entire value is being identical across every repo and reviewer, so a reworded variant would be unsearchable — pinned by test.Mention only humans.
mentionableAuthorreturns a handle only for a real account. Most PRs on a hive fleet are agent-authored, so the author is an App (app/<name>) or a bot (<name>[bot]); @-mentioning either notifies nobody while still reading as though the review were routed. That false signal is worse than no mention, because it implies someone is already on it. For those authors the prompt explicitly suppresses the mention.Report review limits. Asks the reviewer to name what it could not judge — missing context, unfamiliar subsystem, untestable change — and use
requires_humanrather than approving around it. This is the "say when the agent wasn't capable enough" half.Scope
Routing rides the existing
review.post_commentsswitch, so a hive that has not opted into commenting sees no prompt change.TestRoutingIsOptInWithPublishpins that, and the pre-existing strict-superset test still holds.Tests
5 new tests (12 total in the file, all passing): marker pinned to the blocking verdicts; bot/app/empty authors never mentioned and get the explicit suppression line; human authors mentioned with no
@@double-prefix; review-limits instruction present; routing absent by default.go vetclean,gofmtclean.