Route workflow approval checks to reviewers and maintainers - #343
Merged
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The implementation consistently propagates the new blocker category and includes focused coverage across affected behavior.
Pull request overview
Routes workflow-approval blockers to reviewers or maintainers while preserving genuine CI failures for authors.
Changes:
- Separates
ACTION_REQUIREDchecks from CI failures and persists the new fact. - Updates routing, blocker presentation, and dashboard status.
- Adds coverage for normalization, routing, rendering, and state regeneration.
File summaries
| File | Description |
|---|---|
utils.py |
Distinguishes settled checks from unreported checks. |
dashboard_contracts.py |
Adds the maintainer-action count. |
github_cli.py |
Classifies ACTION_REQUIRED checks separately. |
pull_request_evaluation.py |
Computes the new check fact. |
routing_decision.py |
Allows reported approval blockers to route onward. |
route_presentation.py |
Excludes approval blockers from unreported gates. |
render.py |
Displays approval blockers as failed CI. |
pr_status_comment.py |
Explains required maintainer action. |
dashboard.py |
Documents the new fact semantics. |
state.py |
Persists the fact and advances state revisions. |
test_github_cli.py |
Tests check classification. |
test_pull_request_source.py |
Tests source normalization. |
test_dashboard.py |
Tests evaluation and integrated routing. |
test_routing_decision.py |
Tests approval-based routes and mixed blockers. |
test_render.py |
Tests the dashboard CI blocker indicator. |
test_pr_status_comment.py |
Tests blocker messaging. |
test_state.py |
Tests persistence and version regeneration. |
Review details
- Files reviewed: 17/17 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:52 UTC Merge when ready. Status above doesn't look right?
|
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9dc6c323-cc68-4ec7-a881-b441bc269583
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9dc6c323-cc68-4ec7-a881-b441bc269583
trask
changed the title
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Optional action-required checks are incorrectly presented as failed checks, and routing documentation is stale.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 17/17 changed files
- Comments generated: 2
- Review effort level: Balanced
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9dc6c323-cc68-4ec7-a881-b441bc269583
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Generic ACTION_REQUIRED checks are incorrectly assumed to require maintainer write access.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 18/18 changed files
- Comments generated: 1
- Review effort level: Balanced
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9dc6c323-cc68-4ec7-a881-b441bc269583
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9dc6c323-cc68-4ec7-a881-b441bc269583
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The implementation consistently handles classification, routing, presentation, persistence, and associated edge cases with focused test coverage.
Review details
- Files reviewed: 18/18 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Generic action-required checks can bypass the Copilot review request delivery-time failure guard.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 18/18 changed files
- Comments generated: 1
- Review effort level: Balanced
Copilot comment: The new generic `action_required` bucket is not treated as a failure by `copilot_review.stale_request_reason()`, which only rejects `fail` and `cancel`. Because check state is intentionally excluded from the Copilot request fingerprint, a request queued while this check was pending can still be delivered after it becomes `ACTION_REQUIRED`, even though this PR now routes that state to the author as a required-check failure. Include this bucket in the delivery-time failure check (and cover the pending-to-action-required case). Analysis: `stale_request_reason()` now treats `action_required` like the existing failure buckets at delivery time. The regression test keeps the request fingerprint unchanged to model a request recorded while the check was pending, then supplies an `action_required` snapshot. Upsides: Copilot review requests are discarded when a required check changes from pending to an unknown-owner action. Delivery behavior now matches dashboard routing. Downsides: Generic action-required checks are handled conservatively even when their actual owner is not the pull request author. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The classification, routing, persistence, presentation, documentation, and regression coverage are consistent and complete.
Review details
- Files reviewed: 20/20 changed files
- Comments generated: 0 new
- Review effort level: Balanced
…ccurately Review finding: Bumping `DASHBOARD_STATE_VERSION` to 14 makes `.github/scripts/pull-request-dashboard/CONTEXT.md` wrong. Line 33 of that file says `state.py`'s "dashboard facts, stored-result, and state codecs translate the immutable contracts to the version 13 `dashboard-state.json` shape". That sentence was accurate at the base commit, where the constant was 13, and this PR is what makes it false: `encode_dashboard_state` now writes `"version": 14`. CONTEXT.md is this directory's own architecture document, so a reader sees a version number that no longer matches the code. The directory has kept this sentence in step with the constant before; commit 257477f updated it from 12 to 13 in the same change that bumped the constant. Fix: update line 33 of CONTEXT.md to name version 14. Review finding: Dropping `compatible_versions=(11, 12)` from `load_dashboard_state_cache` makes the name of the existing test `test_version_eleven_dashboard_state_migrates_to_current_shape` (`.github/scripts/pull-request-dashboard/test_state.py`) wrong. A version 11 dashboard state file is now rejected by `load_state_file` and regenerated, so it never migrates to the current shape. The test body only round-trips `decode_dashboard_state` and `encode_dashboard_state`, which never consult the version, so it still passes while claiming behavior this PR removed. That is confusing next to the new `test_version_thirteen_dashboard_state_is_regenerated`, which asserts the opposite outcome for a newer version. Fix: rename that test to describe what it actually covers, such as the decoder accepting a legacy stored facts payload and the encoder rewriting it at the current version. Analysis: both findings come from one decision in this change, which is to regenerate the dashboard state under version 14 instead of migrating older files. `encode_dashboard_state` stamps `DASHBOARD_STATE_VERSION` into every file it writes, so the shape the codecs produce is now version 14, and CONTEXT.md is the only architecture document that names that number. `load_dashboard_state_cache` no longer passes `compatible_versions`, so `load_state_file` rejects any file whose `version` is not 14 and returns `None`, and the dashboard rebuilds the state from GitHub. Version 11 therefore no longer reaches the codecs through a load at all. The renamed test never exercised loading; it asserts that `decode_dashboard_state` accepts an older stored payload and that `encode_dashboard_state` rewrites it at the current version, which is still worth covering and still passes. The new name says that, and it no longer collides with `test_version_thirteen_dashboard_state_is_regenerated`, which asserts that a newer stored version is discarded. The payload keeps `"version": 11` because the point is that the decoder ignores the stored version. Upsides: a reader of CONTEXT.md sees the version the codecs actually write. The two version tests now read as a pair, one for a legacy payload the decoder still accepts and one for a stored version the loader now discards, instead of contradicting each other. Downsides: No material downside identified. Neither edit changes behavior, and `python -m unittest test_state` passes with all 35 tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
trask
changed the title
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The CI cell hides running required checks when a workflow approval is also outstanding.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
.github/scripts/pull-request-dashboard/render.py:111
- When a workflow approval coexists with another running required check, this branch returns
🔐before examiningci_pending_count. The route is still held on the author until that check completes, but the CI cell hides the running gate and appears to show only a maintainer-owned blocker. Render both states (for example,⏳ 🔐) and cover this mixed case.
- Files reviewed: 21/21 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The ownership distinction, routing behavior, persistence, presentation, and edge cases are consistently implemented and tested.
Review details
- Files reviewed: 21/21 changed files
- Comments generated: 0 new
- Review effort level: Balanced
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
Combine workflow approval routing with automation author routing and advance the dashboard state format to version 16. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4a473815-a7a5-4cf1-bffb-adbb719c1cb1
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.
Routes GitHub Actions workflow approval checks to reviewers while pull request approvals are outstanding, then to maintainers once the pull request has enough approvals. Authors no longer receive blockers that only someone with repository write access can clear.
Only
ACTION_REQUIREDchecks from the GitHub Actions app with workflow-run metadata use this routing. Checks with unknown ownership and cases that also have a required-check failure still route to the author.The dashboard marks workflow approvals with 🔐. Status comments tell maintainers to unblock the checks.