Separate automation author routing from approval policy - #342
Merged
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The focused implementation matches the stated routing behavior and includes coverage at evaluation and routing boundaries.
Pull request overview
Recognizes known maintenance bots across GitHub identity formats so automation PRs route to reviewers and maintainers rather than their bot author.
Changes:
- Canonicalizes app and bot login wrappers.
- Adds
opentelemetrybotas a maintenance identity. - Tests routing with failing CI, author feedback, and approval progression.
File summaries
| File | Description |
|---|---|
pull_request_evaluation.py |
Normalizes identities and classifies maintenance bots. |
test_pull_request_evaluation.py |
Covers identity forms and end-to-end routing. |
test_routing_decision.py |
Verifies bot PRs never route to their author. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Pull request dashboard statusWaiting on maintainers · refreshed 2026-09-04 06:51 UTC Merge when ready. Status above doesn't look right?
|
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 41074282-3988-4c4d-829c-fcefcebfcf69
trask
changed the title
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 41074282-3988-4c4d-829c-fcefcebfcf69
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Bare renovate and otelbot users are incorrectly granted maintenance-bot approval policy.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 1
- Review effort level: Balanced
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 41074282-3988-4c4d-829c-fcefcebfcf69
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The implementation consistently separates author capability from approval policy and includes targeted regression coverage.
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 41074282-3988-4c4d-829c-fcefcebfcf69
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The dashboard state version must be incremented to prevent older workers from restoring obsolete routing during rollout.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 1
- Review effort level: Balanced
Advance dashboard state to version 15 without reusing the independent version 14 schema. Migrate production versions through author identity inference and regenerate unknown version 14 state. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Compatible legacy caches can retain an author route for unattended automation until reevaluation.
Review details
Suppressed comments (1)
.github/scripts/pull-request-dashboard/state.py:819
- Accepting versions 11–13 preserves each cached
StoredDashboardResult.routeunchanged. For example, the new v13 fixture decodesauthor_can_act=Falseforapp/dependabotbut still leaves its stored route asauthor; dashboard rendering, status comments, and nudge delivery consume that route directly, so work can continue to be routed to the unavailable bot until backfill reevaluates that PR. Please migrate cachedauthorroutes for unattended authors to at leastapprover, or regenerate these older versions instead of accepting them.
compatible_versions=(11, 12, 13),
- Files reviewed: 12/12 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Discard compatible cached results that cannot reconcile an author route with the inferred or explicit author capability. Preserve human and explicit actionable-author routes for migration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The policy separation, migration behavior, and key automation identity cases are consistently implemented and covered by focused tests.
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The routing documentation incorrectly omits the Copilot-delegation exception.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 2
- Review effort level: Balanced
Copilot comment: “Automation-authored PRs never route to their author” incorrectly includes Copilot-delegated PRs, whose effective author is deliberately recovered and can receive the author route. Qualify this as unattended automation and document the delegation exception. Copilot comment: This states that every automation-authored PR avoids the author route, but Copilot-delegated PRs intentionally route to the recovered human author. Please distinguish unattended automation from delegated Copilot authors so the documented policy matches the implementation and PR description. Analysis: Both documents described the rule too broadly. The implementation excludes unattended automation from author routing, but it recovers a human assignee or the first commit's human committer for a Copilot-delegated pull request. The revised text states both cases. Upsides: The policy documentation now matches the author recovery logic and tells operators when a Copilot-authored pull request can return to a person. Downsides: The documentation now includes the two inputs used to recover the human author. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Review finding: This PR deletes the comment that recorded how GitHub reports Copilot logins: `gh pr view`'s `author` field uses the `app/<slug>` form, while the commits endpoint's `committer.login` field can return the bare `copilot` slug. The replacement constants still depend on that knowledge. `_COPILOT_PR_AUTHOR_IDENTITIES` keeps both `copilot-swe-agent` and the bare `copilot`, and `_COPILOT_COMMITTER_IDENTITIES` exists as a separate one-element set only because of the committer shape, yet nothing in the file explains either fact any more. A reader cannot tell why the two sets differ or why bare `copilot` is listed as a pull request author identity, and could reasonably merge them or drop the bare entry. Fix: restore a short comment above these constants stating the two API shapes and why the committer set stays separate. Analysis: The two Copilot slugs come from two different GitHub endpoints, and no amount of reading the code recovers that. `normalize_author_identity` strips the `app/` prefix and the `[bot]` suffix, which explains the shape of each entry but not why `copilot-swe-agent` and `copilot` are both author identities while only `copilot` is a committer identity. The restored comment records the endpoint behavior, says which set covers which endpoint, and notes that the entries are already normalized. Upsides: A reader can tell why the two sets differ and will not merge them or drop the bare `copilot` entry, which would let a Copilot-authored PR be recovered as if a human had written it. Downsides: No material downside identified. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
trask
marked this pull request as ready for review
September 2, 2026 22:00
trask
marked this pull request as draft
September 2, 2026 23:03
trask
marked this pull request as ready for review
September 3, 2026 15:15
maryliag
approved these changes
Sep 4, 2026
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
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.
Prevents unattended automation PRs from routing dashboard work to an author that cannot respond. Copilot-delegated PRs can still route to a human recovered from an assignee or the first commit's committer.
Dependabot, Renovate, otelbot, and opentelemetrybot keep the one-approval maintenance threshold. Other automation uses the repository's configured approval count.
Dashboard state advances to version 15. When the dashboard loads state from versions 11 through 13, it derives a missing
author_can_actvalue and drops stale author routes that conflict with it. Version 14 remains reserved for #343 and is regenerated.