From f8d49359b5b63767a35da350afa35f556ceec479 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Wed, 2 Sep 2026 09:51:43 -0700 Subject: [PATCH 1/9] Route workflow approval checks to maintainers Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../pull-request-dashboard/dashboard.py | 11 ++- .../dashboard_contracts.py | 1 + .../pull-request-dashboard/github_cli.py | 8 ++- .../pr_status_comment.py | 34 ++++++++- .../pull_request_evaluation.py | 10 +++ .../scripts/pull-request-dashboard/render.py | 11 ++- .../route_presentation.py | 5 +- .../routing_decision.py | 13 +++- .../scripts/pull-request-dashboard/state.py | 13 +++- .../pull-request-dashboard/test_dashboard.py | 70 ++++++++++++++++--- .../pull-request-dashboard/test_github_cli.py | 39 +++++++++++ .../test_pr_status_comment.py | 54 ++++++++++++++ .../test_pull_request_source.py | 12 +++- .../pull-request-dashboard/test_render.py | 11 +++ .../test_routing_decision.py | 47 +++++++++++++ .../pull-request-dashboard/test_state.py | 20 +++++- .../scripts/pull-request-dashboard/utils.py | 16 ++++- 17 files changed, 346 insertions(+), 29 deletions(-) diff --git a/.github/scripts/pull-request-dashboard/dashboard.py b/.github/scripts/pull-request-dashboard/dashboard.py index b42dd8d8f7ff..f3287a61a5b8 100644 --- a/.github/scripts/pull-request-dashboard/dashboard.py +++ b/.github/scripts/pull-request-dashboard/dashboard.py @@ -114,6 +114,11 @@ fetched. ci_failing_since str (iso) Earliest completion time among current required failures. + ci_maintainer_action_required_count + int Required checks that need a + maintainer-owned permission + action; absent when checks + could not be fetched. ci_pending_count int Merge-blocking checks only; absent when checks could not be fetched, and excludes required @@ -191,9 +196,9 @@ Copilot review are still outstanding. required_checks_settled bool Every required check has - reported on the current head, - so the computed route is not - provisional. + completed without needing a + maintainer-owned permission + action. route_held_since str (iso) When the gates first kept this PR off its reviewers on this head. Cleared once every gate diff --git a/.github/scripts/pull-request-dashboard/dashboard_contracts.py b/.github/scripts/pull-request-dashboard/dashboard_contracts.py index 566d7de033f4..17937bd41ec0 100644 --- a/.github/scripts/pull-request-dashboard/dashboard_contracts.py +++ b/.github/scripts/pull-request-dashboard/dashboard_contracts.py @@ -101,6 +101,7 @@ class DashboardFacts: last_approver_activity_at: str = "" ci_failing_count: int | None = None ci_failing_since: str | None = None + ci_maintainer_action_required_count: int | None = None ci_pending_count: int | None = None non_blocking_check_failures: tuple[str, ...] = () copilot_first_review_missing_since: str | None = None diff --git a/.github/scripts/pull-request-dashboard/github_cli.py b/.github/scripts/pull-request-dashboard/github_cli.py index bd2e95561632..86c415d80f84 100644 --- a/.github/scripts/pull-request-dashboard/github_cli.py +++ b/.github/scripts/pull-request-dashboard/github_cli.py @@ -367,7 +367,9 @@ def check_bucket(state: str) -> str: return "pass" if state in ("SKIPPED", "NEUTRAL"): return "skipping" - if state in ("ERROR", "FAILURE", "TIMED_OUT", "ACTION_REQUIRED", "STARTUP_FAILURE"): + if state == "ACTION_REQUIRED": + return "maintainer_action_required" + if state in ("ERROR", "FAILURE", "TIMED_OUT", "STARTUP_FAILURE"): return "fail" if state == "CANCELLED": return "cancel" @@ -488,7 +490,9 @@ def gh_pr_check_rollup( "non_blocking_failures": [ check for check, is_required in checks - if not is_required and check.get("bucket") in ("fail", "cancel") + if not is_required + and check.get("bucket") + in ("fail", "cancel", "maintainer_action_required") ], "code_scanning": [ check diff --git a/.github/scripts/pull-request-dashboard/pr_status_comment.py b/.github/scripts/pull-request-dashboard/pr_status_comment.py index 90f598f401e1..0157c316929e 100644 --- a/.github/scripts/pull-request-dashboard/pr_status_comment.py +++ b/.github/scripts/pull-request-dashboard/pr_status_comment.py @@ -216,6 +216,18 @@ def author_body( return [fallback_next_step] +def maintainer_action_required_summary(count: int) -> str: + if count == 1: + return ( + "1 required check needs a maintainer to approve or otherwise " + "unblock its workflow." + ) + return ( + f"{count} required checks need a maintainer to approve or otherwise " + "unblock their workflows." + ) + + def is_terminal_pr(pr: dict[str, Any]) -> bool: return bool(pr.get("merged")) or (pr.get("state") or "").lower() == "closed" @@ -235,6 +247,9 @@ def render_status_comment( top_level_feedback_urls = facts.author_action_top_level_feedback_urls feedback_count = len(review_thread_urls) + len(top_level_feedback_urls) failing_count = facts.ci_failing_count or 0 + maintainer_action_required_count = ( + facts.ci_maintainer_action_required_count or 0 + ) non_blocking_check_failures = facts.non_blocking_check_failures override_route = "" @@ -289,7 +304,14 @@ def render_status_comment( body = ( ["Resolve merge conflicts, then merge when ready."] if conflicted and route is DashboardRoute.MAINTAINER - else [next_step] + else ( + ["Approve or otherwise unblock the required workflow checks."] + if ( + route is DashboardRoute.MAINTAINER + and maintainer_action_required_count + ) + else [next_step] + ) ) abandoned_gates = ( abandoned_gate_note(facts) @@ -317,6 +339,16 @@ def render_status_comment( body.extend(["", f"**{label}:** {names}"]) if conflicted and route is not DashboardRoute.MAINTAINER: body.extend(["", "**Also blocked by:** Merge conflicts."]) + if maintainer_action_required_count: + body.extend([ + "", + ( + "**Maintainer action required:** " + + maintainer_action_required_summary( + maintainer_action_required_count + ) + ), + ]) lines = [ STATUS_MARKER, diff --git a/.github/scripts/pull-request-dashboard/pull_request_evaluation.py b/.github/scripts/pull-request-dashboard/pull_request_evaluation.py index 476ad1d3cde5..44487c80d92c 100644 --- a/.github/scripts/pull-request-dashboard/pull_request_evaluation.py +++ b/.github/scripts/pull-request-dashboard/pull_request_evaluation.py @@ -170,6 +170,11 @@ def _compute_facts( for check in checks or () if check.bucket == "pending" ] + maintainer_action_required = [ + check + for check in checks or () + if check.bucket == "maintainer_action_required" + ] failing_timestamps = [parse_ts(check.completed_at) for check in failing] failing_timestamps = [ts for ts in failing_timestamps if ts is not None] created_ts = parse_ts(pr.created_at) @@ -248,6 +253,11 @@ def _compute_facts( if failing_timestamps else None ), + ci_maintainer_action_required_count=( + len(maintainer_action_required) + if checks is not None + else None + ), ci_pending_count=len(pending) if checks is not None else None, non_blocking_check_failures=non_blocking_check_failures, ) diff --git a/.github/scripts/pull-request-dashboard/render.py b/.github/scripts/pull-request-dashboard/render.py index 69d13ea4cba6..80e37ec21138 100644 --- a/.github/scripts/pull-request-dashboard/render.py +++ b/.github/scripts/pull-request-dashboard/render.py @@ -91,9 +91,16 @@ def render_draft_pr_section( def ci_cell(facts: DashboardFacts) -> str: - if facts.ci_failing_count is None and facts.ci_pending_count is None: + if ( + facts.ci_failing_count is None + and facts.ci_maintainer_action_required_count is None + and facts.ci_pending_count is None + ): return "?" - if (facts.ci_failing_count or 0) > 0: + if ( + (facts.ci_failing_count or 0) > 0 + or (facts.ci_maintainer_action_required_count or 0) > 0 + ): return "❌" if (facts.ci_pending_count or 0) > 0: return "⏳" diff --git a/.github/scripts/pull-request-dashboard/route_presentation.py b/.github/scripts/pull-request-dashboard/route_presentation.py index 824b196c166a..9b964d56f8c6 100644 --- a/.github/scripts/pull-request-dashboard/route_presentation.py +++ b/.github/scripts/pull-request-dashboard/route_presentation.py @@ -1,6 +1,7 @@ from __future__ import annotations from dashboard_contracts import DashboardFacts, DashboardRoute +from utils import required_checks_unreported ROUTE_PRESENTATION = { @@ -73,7 +74,7 @@ def unreported_gate_phrase(facts: DashboardFacts) -> str: # findings holds it but has reported, so naming it would send the reader # after a gate that arrived. gates = [] - if not facts.required_checks_settled: + if required_checks_unreported(facts): gates.append("the required status checks") if facts.copilot_review_unreported: gates.append("the Copilot review") @@ -87,7 +88,7 @@ def held_gate_phrase(facts: DashboardFacts) -> str: # review, which has already arrived. A held route always has one of these, # so the phrase is never empty while the pull request is held. gates = [] - if not facts.required_checks_settled: + if required_checks_unreported(facts): gates.append("the required status checks") if facts.copilot_review_unreported: gates.append("the Copilot review") diff --git a/.github/scripts/pull-request-dashboard/routing_decision.py b/.github/scripts/pull-request-dashboard/routing_decision.py index b53f5ad373d1..c66a92c4085f 100644 --- a/.github/scripts/pull-request-dashboard/routing_decision.py +++ b/.github/scripts/pull-request-dashboard/routing_decision.py @@ -15,7 +15,13 @@ set_copilot_review_request_needed, ) from dashboard_contracts import DashboardFacts, DashboardRoute -from utils import format_ts, parse_ts, required_checks_settled, utc_now +from utils import ( + format_ts, + parse_ts, + required_checks_settled, + required_checks_unreported, + utc_now, +) @dataclass(frozen=True) @@ -166,11 +172,12 @@ def _hold_route_until_gates_settle( ), required_checks_settled=required_checks_settled(facts), ) + checks_unreported = required_checks_unreported(facts) gates_outstanding = gates_enabled and ( - not facts.required_checks_settled or facts.copilot_review_outstanding + checks_unreported or facts.copilot_review_outstanding ) unreported_gates = gates_enabled and ( - not facts.required_checks_settled or facts.copilot_review_unreported + checks_unreported or facts.copilot_review_unreported ) would_hold = _route_progress(route) > _route_progress(effective_previous_route) facts = _set_gate_hold_clock( diff --git a/.github/scripts/pull-request-dashboard/state.py b/.github/scripts/pull-request-dashboard/state.py index 98320e3dccf1..3809a194b2a5 100644 --- a/.github/scripts/pull-request-dashboard/state.py +++ b/.github/scripts/pull-request-dashboard/state.py @@ -35,7 +35,7 @@ # current vector, ordinary state loaders may regenerate mismatched disposable # caches. Every constant ending in _STATE_VERSION or _REVISION is included. # dashboard-state.json: accepted PR routing results and backfill readiness. -DASHBOARD_STATE_VERSION = 13 +DASHBOARD_STATE_VERSION = 14 # backfill-state.json: round-robin cursor used by full dashboard refreshes. BACKFILL_STATE_VERSION = 3 # notification-state.json: pending and delivered Slack notification records. @@ -48,7 +48,7 @@ STATUS_COMMENT_ROLLOUT_STATE_VERSION = 2 # Rendered status-comment behavior. Increment when existing comments need to # adopt a change; hourly runs durably roll it out to all open PRs. -STATUS_COMMENT_REVISION = 16 +STATUS_COMMENT_REVISION = 17 INITIAL_BACKFILL_COMPLETE_KEY = "initial_backfill_complete" _state_dir: Path | None = None @@ -553,6 +553,10 @@ def decode_dashboard_facts(value: Any) -> DashboardFacts: value.get("ci_failing_since"), "facts.ci_failing_since", ), + ci_maintainer_action_required_count=_optional_integer( + value.get("ci_maintainer_action_required_count"), + "facts.ci_maintainer_action_required_count", + ), ci_pending_count=_optional_integer( value.get("ci_pending_count"), "facts.ci_pending_count", @@ -674,6 +678,10 @@ def encode_dashboard_facts(facts: DashboardFacts) -> dict[str, Any]: stored["ci_failing_count"] = facts.ci_failing_count if facts.ci_failing_since is not None: stored["ci_failing_since"] = facts.ci_failing_since + if facts.ci_maintainer_action_required_count is not None: + stored["ci_maintainer_action_required_count"] = ( + facts.ci_maintainer_action_required_count + ) if facts.ci_pending_count is not None: stored["ci_pending_count"] = facts.ci_pending_count if facts.non_blocking_check_failures: @@ -808,7 +816,6 @@ def load_dashboard_state_cache() -> DashboardState | None: state = load_state_file( dashboard_state_path(), DASHBOARD_STATE_VERSION, - compatible_versions=(11, 12), ) if state is None: return None diff --git a/.github/scripts/pull-request-dashboard/test_dashboard.py b/.github/scripts/pull-request-dashboard/test_dashboard.py index 857b7d9810e4..c226435a49e1 100644 --- a/.github/scripts/pull-request-dashboard/test_dashboard.py +++ b/.github/scripts/pull-request-dashboard/test_dashboard.py @@ -1819,15 +1819,21 @@ def test_non_blocking_check_failures_use_deterministic_casefold_tiebreaker(self) def test_required_check_buckets_control_ci_facts(self) -> None: cases = ( - ("TIMED_OUT", "fail", 1, 0), - ("ACTION_REQUIRED", "fail", 1, 0), - ("STARTUP_FAILURE", "fail", 1, 0), - ("CANCELLED", "cancel", 1, 0), - ("IN_PROGRESS", "pending", 0, 1), - ("SKIPPED", "skipping", 0, 0), - ("SUCCESS", "pass", 0, 0), - ) - for state, bucket, failing, pending in cases: + ("TIMED_OUT", "fail", 1, 0, 0), + ( + "ACTION_REQUIRED", + "maintainer_action_required", + 0, + 1, + 0, + ), + ("STARTUP_FAILURE", "fail", 1, 0, 0), + ("CANCELLED", "cancel", 1, 0, 0), + ("IN_PROGRESS", "pending", 0, 0, 1), + ("SKIPPED", "skipping", 0, 0, 0), + ("SUCCESS", "pass", 0, 0, 0), + ) + for state, bucket, failing, maintainer_action, pending in cases: with self.subTest(state=state, bucket=bucket): facts = evaluation_facts( { @@ -1849,6 +1855,10 @@ def test_required_check_buckets_control_ci_facts(self) -> None: ) self.assertEqual(failing, facts.ci_failing_count) + self.assertEqual( + maintainer_action, + facts.ci_maintainer_action_required_count, + ) self.assertEqual(pending, facts.ci_pending_count) self.assertEqual( ("workflow-notification",), @@ -1887,6 +1897,48 @@ def test_override_command_does_not_clear_required_check_failures(self) -> None: self.assertEqual(3, facts.ci_failing_count) self.assertEqual("2026-07-17T01:00:00+00:00", facts.ci_failing_since) + @patch("pull_request_evaluation.fetch_pull_request_source") + def test_permission_owned_blockers_route_like_audited_pull_requests( + self, + fetch_source: Mock, + ) -> None: + cases = ( + (4998, "opentelemetry-python-contrib", (), DashboardRoute.APPROVER), + ( + 3706, + "opentelemetry-js-contrib", + (review_source(state="APPROVED", body=""),), + DashboardRoute.MAINTAINER, + ), + ) + for number, repository, reviews, expected_route in cases: + with self.subTest(number=number, repository=repository): + fetch_source.return_value = pull_request_source( + pull_request=pull_request_metadata( + number=number, + title=f"{repository} workflow approval", + ), + reviews=reviews, + checks=(check_source( + name="workflow approval", + state="ACTION_REQUIRED", + bucket="maintainer_action_required", + ),), + ) + + result = evaluate_pr({"number": number}) + + self.assertIsInstance(result, EvaluationSuccess) + assert isinstance(result, EvaluationSuccess) + self.assertEqual(expected_route, result.route) + self.assertEqual(0, result.facts.ci_failing_count) + self.assertEqual( + 1, + result.facts.ci_maintainer_action_required_count, + ) + self.assertFalse(result.facts.required_checks_settled) + self.assertFalse(result.facts.route_held_for_gates) + class ActivityFactsIntegrationTest(unittest.TestCase): def test_formats_activity_clocks_and_clamps_overall_activity_to_creation( diff --git a/.github/scripts/pull-request-dashboard/test_github_cli.py b/.github/scripts/pull-request-dashboard/test_github_cli.py index 402c711b636d..b96df7cde414 100644 --- a/.github/scripts/pull-request-dashboard/test_github_cli.py +++ b/.github/scripts/pull-request-dashboard/test_github_cli.py @@ -5,6 +5,7 @@ from github_cli import ( TransientGhError, + check_bucket, code_scanning_tools, fetch_pr_issue_comments, fetch_pr_reviews, @@ -130,6 +131,44 @@ def test_follows_pagination(self) -> None: class GithubCliTest(unittest.TestCase): + def test_action_required_has_a_maintainer_owned_bucket(self) -> None: + self.assertEqual( + "maintainer_action_required", + check_bucket("ACTION_REQUIRED"), + ) + self.assertEqual("fail", check_bucket("FAILURE")) + self.assertEqual("pending", check_bucket("IN_PROGRESS")) + + @patch("github_cli.gh_graphql") + def test_optional_action_required_check_remains_non_blocking( + self, + graphql, + ) -> None: + graphql.return_value = _rollup_page([{ + "__typename": "CheckRun", + "name": "optional-deploy", + "status": "COMPLETED", + "conclusion": "ACTION_REQUIRED", + "url": "https://github.com/open-telemetry/example/runs/1", + "isRequired": False, + }]) + + rollup = gh_pr_check_rollup( + "open-telemetry/example", + "PR_id", + ["optional-*"], + ) + + assert rollup is not None + self.assertEqual([], rollup["required"]) + self.assertEqual( + ["maintainer_action_required"], + [ + check["bucket"] + for check in rollup["non_blocking_failures"] + ], + ) + @patch("github_cli.run_gh_json") def test_pr_view_fetches_body_for_routing_freshness(self, run_json) -> None: run_json.return_value = {"mergeable": "MERGEABLE"} diff --git a/.github/scripts/pull-request-dashboard/test_pr_status_comment.py b/.github/scripts/pull-request-dashboard/test_pr_status_comment.py index 9a251567f793..47d6b65df314 100644 --- a/.github/scripts/pull-request-dashboard/test_pr_status_comment.py +++ b/.github/scripts/pull-request-dashboard/test_pr_status_comment.py @@ -359,6 +359,60 @@ def test_waiting_on_author_names_required_ci_failure(self) -> None: self.assertNotIn("### Review feedback", body) self.assertNotIn(pr_status_comment.RESPONSE_EXAMPLES, body) + def test_waiting_on_reviewers_names_maintainer_owned_blocker(self) -> None: + body = pr_status_comment.render_status_comment( + self.pr(), + status_result( + DashboardRoute.APPROVER, + ci_failing_count=0, + ci_maintainer_action_required_count=1, + ci_pending_count=0, + ), + ) + + self.assertIn("**Waiting on reviewers** · refreshed ", body) + self.assertIn("Review the latest changes.", body) + self.assertIn( + "**Maintainer action required:** 1 required check needs a maintainer " + "to approve or otherwise unblock its workflow.", + body, + ) + self.assertNotIn("status check is failing", body) + + def test_waiting_on_maintainers_leads_with_permission_action(self) -> None: + body = pr_status_comment.render_status_comment( + self.pr(), + status_result( + DashboardRoute.MAINTAINER, + approval_count=1, + ci_failing_count=0, + ci_maintainer_action_required_count=2, + ci_pending_count=0, + ), + ) + + self.assertIn("Approve or otherwise unblock the required workflow checks.", body) + self.assertIn( + "**Maintainer action required:** 2 required checks need a maintainer " + "to approve or otherwise unblock their workflows.", + body, + ) + self.assertNotIn("Merge when ready.", body) + + def test_mixed_failure_and_permission_action_names_both_blockers(self) -> None: + body = pr_status_comment.render_status_comment( + self.pr(), + status_result( + DashboardRoute.AUTHOR, + ci_failing_count=1, + ci_maintainer_action_required_count=1, + ci_pending_count=0, + ), + ) + + self.assertIn("Investigate required status check failures.", body) + self.assertIn("**Maintainer action required:**", body) + def test_waiting_on_author_names_merge_conflicts(self) -> None: body = pr_status_comment.render_status_comment( self.pr(), diff --git a/.github/scripts/pull-request-dashboard/test_pull_request_source.py b/.github/scripts/pull-request-dashboard/test_pull_request_source.py index 5051d0253574..c874fb6ccc77 100644 --- a/.github/scripts/pull-request-dashboard/test_pull_request_source.py +++ b/.github/scripts/pull-request-dashboard/test_pull_request_source.py @@ -136,7 +136,13 @@ def test_normalizes_mixed_gh_rest_and_graphql_shapes(self) -> None: "state": "SUCCESS", "bucket": "pass", "integration_id": 1, - } + }, + { + "name": "workflow approval", + "state": "ACTION_REQUIRED", + "bucket": "maintainer_action_required", + "integration_id": 2, + }, ], "non_blocking_check_failures": [ { @@ -179,6 +185,10 @@ def test_normalizes_mixed_gh_rest_and_graphql_shapes(self) -> None: .user_logins, ) self.assertEqual("pass", source.checks[0].bucket) + self.assertEqual( + "maintainer_action_required", + source.checks[1].bucket, + ) self.assertEqual("optional", source.non_blocking_failures[0].name) def test_normalizes_bot_and_human_actor_cases(self) -> None: diff --git a/.github/scripts/pull-request-dashboard/test_render.py b/.github/scripts/pull-request-dashboard/test_render.py index 44704ff59d7f..f553c053a77f 100644 --- a/.github/scripts/pull-request-dashboard/test_render.py +++ b/.github/scripts/pull-request-dashboard/test_render.py @@ -8,6 +8,7 @@ stored_dashboard_result, ) from render import ( + ci_cell, render_draft_pr_section, render_pr_tables, reviewers_cell_text, @@ -15,6 +16,16 @@ class RenderTest(unittest.TestCase): + def test_maintainer_owned_check_action_is_a_ci_blocker(self) -> None: + self.assertEqual( + "❌", + ci_cell(dashboard_facts( + ci_failing_count=0, + ci_maintainer_action_required_count=1, + ci_pending_count=0, + )), + ) + def test_reviewer_legend_includes_top_level_feedback(self) -> None: markdown = render_pr_tables([], ()) diff --git a/.github/scripts/pull-request-dashboard/test_routing_decision.py b/.github/scripts/pull-request-dashboard/test_routing_decision.py index 6361ad18b4bb..8d93454e03ad 100644 --- a/.github/scripts/pull-request-dashboard/test_routing_decision.py +++ b/.github/scripts/pull-request-dashboard/test_routing_decision.py @@ -286,6 +286,53 @@ def test_required_check_failure_routes_human_authored_pr_to_author(self) -> None self.assertEqual("2026-07-17T01:00:00+00:00", outcome.facts.waiting_since) self.assertEqual("ci_failure", outcome.facts.waiting_age_basis) + def test_only_maintainer_owned_check_actions_route_by_approval_count( + self, + ) -> None: + for approval_count, expected in ( + (0, DashboardRoute.APPROVER), + (1, DashboardRoute.MAINTAINER), + ): + with self.subTest(approval_count=approval_count): + outcome = self.resolve({ + "approval_count": approval_count, + "ci_failing_count": 0, + "ci_maintainer_action_required_count": 1, + "ci_pending_count": 0, + "is_maintenance_bot": False, + }) + + self.assertEqual(expected, outcome.route) + self.assertFalse(outcome.facts.required_checks_settled) + self.assertFalse(outcome.facts.route_held_for_gates) + + def test_genuine_failure_wins_over_maintainer_owned_check_action( + self, + ) -> None: + outcome = self.resolve({ + "approval_count": 1, + "ci_failing_count": 1, + "ci_maintainer_action_required_count": 1, + "ci_pending_count": 0, + "is_maintenance_bot": False, + }) + + self.assertEqual(DashboardRoute.AUTHOR, outcome.route) + + def test_pending_checks_still_hold_maintainer_owned_action_route( + self, + ) -> None: + outcome = self.resolve({ + "approval_count": 0, + "ci_failing_count": 0, + "ci_maintainer_action_required_count": 1, + "ci_pending_count": 1, + "is_maintenance_bot": False, + }) + + self.assertEqual(DashboardRoute.AUTHOR, outcome.route) + self.assertTrue(outcome.facts.route_held_for_gates) + def test_reviewer_handoff_is_bound_to_the_current_head(self) -> None: self.assertTrue( reviewer_handoff_active( diff --git a/.github/scripts/pull-request-dashboard/test_state.py b/.github/scripts/pull-request-dashboard/test_state.py index dcb614ea6b10..f663c68893a4 100644 --- a/.github/scripts/pull-request-dashboard/test_state.py +++ b/.github/scripts/pull-request-dashboard/test_state.py @@ -230,6 +230,7 @@ def test_dashboard_facts_codec_round_trip(self) -> None: last_approver_activity_at="2026-08-16T10:00:00Z", ci_failing_count=1, ci_failing_since="2026-08-16T09:00:00Z", + ci_maintainer_action_required_count=2, ci_pending_count=2, non_blocking_check_failures=("CodeQL",), copilot_first_review_missing_since="2026-08-16T08:30:00Z", @@ -343,6 +344,7 @@ def test_dashboard_facts_accepts_null_optional_fields(self) -> None: decode_dashboard_facts({ "ci_failing_count": None, "ci_failing_since": None, + "ci_maintainer_action_required_count": None, "ci_pending_count": None, "copilot_first_review_missing_since": None, "route_held_since": None, @@ -526,11 +528,27 @@ def test_version_eleven_dashboard_state_migrates_to_current_shape(self) -> None: def test_notification_state_version_is_independent(self) -> None: self.assertEqual(BACKFILL_STATE_VERSION, 3) self.assertEqual(NOTIFICATION_STATE_VERSION, 3) - self.assertEqual(DASHBOARD_STATE_VERSION, 13) + self.assertEqual(DASHBOARD_STATE_VERSION, 14) self.assertEqual(STATUS_COMMENT_ROLLOUT_STATE_VERSION, 2) self.assertEqual(AUTHOR_NUDGE_STATE_VERSION, 3) self.assertEqual(COPILOT_REVIEW_REQUEST_STATE_VERSION, 6) + def test_version_thirteen_dashboard_state_is_regenerated(self) -> None: + with ( + tempfile.TemporaryDirectory() as temp_dir, + patch("state._state_dir", Path(temp_dir)), + ): + dashboard_state_path().write_text( + json.dumps({ + "version": 13, + "initial_backfill_complete": True, + "prs": {}, + }), + encoding="utf-8", + ) + + self.assertIsNone(load_dashboard_state_cache()) + def test_author_nudge_state_round_trip(self) -> None: with tempfile.TemporaryDirectory() as temp_dir, patch("state._state_dir", Path(temp_dir)): save_author_nudges({ diff --git a/.github/scripts/pull-request-dashboard/utils.py b/.github/scripts/pull-request-dashboard/utils.py index ec394bfd72a9..d0f1f39dc084 100644 --- a/.github/scripts/pull-request-dashboard/utils.py +++ b/.github/scripts/pull-request-dashboard/utils.py @@ -80,11 +80,23 @@ def is_copilot_reviewer_login(login: str) -> bool: def required_checks_settled(facts: DashboardFacts) -> bool: + if facts.ci_pending_count is None: + return False + return not ( + facts.ci_pending_count + or facts.ci_maintainer_action_required_count + ) + + +def required_checks_unreported(facts: DashboardFacts) -> bool: # A route computed while checks are still running is provisional because a # failure becomes visible only after the check completes. - if facts.ci_pending_count is None: + if facts.required_checks_settled: return False - return not facts.ci_pending_count + return ( + facts.ci_pending_count is None + or facts.ci_pending_count > 0 + ) def format_ts(ts: datetime | None) -> str: From ccf518a59c9d826843b93e3c1509967bbc1591f4 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Wed, 2 Sep 2026 12:38:33 -0700 Subject: [PATCH 2/9] Clarify settled check documentation Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9dc6c323-cc68-4ec7-a881-b441bc269583 --- .github/scripts/pull-request-dashboard/dashboard.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/scripts/pull-request-dashboard/dashboard.py b/.github/scripts/pull-request-dashboard/dashboard.py index f3287a61a5b8..79ab8102b379 100644 --- a/.github/scripts/pull-request-dashboard/dashboard.py +++ b/.github/scripts/pull-request-dashboard/dashboard.py @@ -196,9 +196,9 @@ Copilot review are still outstanding. required_checks_settled bool Every required check has - completed without needing a - maintainer-owned permission - action. + reported a final result, and + none is waiting for + maintainer action. route_held_since str (iso) When the gates first kept this PR off its reviewers on this head. Cleared once every gate From 40b821bf19d3486fba1055ab60e919b12c20dd75 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Wed, 2 Sep 2026 12:51:38 -0700 Subject: [PATCH 3/9] Distinguish permission-blocked checks Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9dc6c323-cc68-4ec7-a881-b441bc269583 --- .../pr_status_comment.py | 14 +++++------ .../scripts/pull-request-dashboard/render.py | 19 +++++++++++---- .../scripts/pull-request-dashboard/state.py | 2 +- .../test_pr_status_comment.py | 10 ++++---- .../pull-request-dashboard/test_render.py | 23 +++++++++++++++++-- 5 files changed, 49 insertions(+), 19 deletions(-) diff --git a/.github/scripts/pull-request-dashboard/pr_status_comment.py b/.github/scripts/pull-request-dashboard/pr_status_comment.py index 0157c316929e..f38573ae84ae 100644 --- a/.github/scripts/pull-request-dashboard/pr_status_comment.py +++ b/.github/scripts/pull-request-dashboard/pr_status_comment.py @@ -216,15 +216,15 @@ def author_body( return [fallback_next_step] -def maintainer_action_required_summary(count: int) -> str: +def write_access_required_summary(count: int) -> str: if count == 1: return ( - "1 required check needs a maintainer to approve or otherwise " - "unblock its workflow." + "1 required check needs action from someone with write access " + "to this repository." ) return ( - f"{count} required checks need a maintainer to approve or otherwise " - "unblock their workflows." + f"{count} required checks need action from someone with write access " + "to this repository." ) @@ -343,8 +343,8 @@ def render_status_comment( body.extend([ "", ( - "**Maintainer action required:** " - + maintainer_action_required_summary( + "**Write access required:** " + + write_access_required_summary( maintainer_action_required_count ) ), diff --git a/.github/scripts/pull-request-dashboard/render.py b/.github/scripts/pull-request-dashboard/render.py index 80e37ec21138..95de0503e92d 100644 --- a/.github/scripts/pull-request-dashboard/render.py +++ b/.github/scripts/pull-request-dashboard/render.py @@ -97,11 +97,16 @@ def ci_cell(facts: DashboardFacts) -> str: and facts.ci_pending_count is None ): return "?" - if ( - (facts.ci_failing_count or 0) > 0 - or (facts.ci_maintainer_action_required_count or 0) > 0 - ): + failing = (facts.ci_failing_count or 0) > 0 + write_access_required = ( + facts.ci_maintainer_action_required_count or 0 + ) > 0 + if failing and write_access_required: + return "❌ 🔐" + if failing: return "❌" + if write_access_required: + return "🔐" if (facts.ci_pending_count or 0) > 0: return "⏳" return "✅" @@ -244,11 +249,17 @@ def render_pr_tables( "⏳ review pending · 💬 open review thread · 📌 top-level feedback needs author action · " "🔴 changes requested." ) + ci_note = ( + "CI column: ✅ passing · ⏳ running · ❌ failing · " + "🔐 write access required." + ) out: list[str] = [ "> [!NOTE]", f"> {grouping_note}", ">", f"> {reviewers_note}", + ">", + f"> {ci_note}", "", ] diff --git a/.github/scripts/pull-request-dashboard/state.py b/.github/scripts/pull-request-dashboard/state.py index 3809a194b2a5..b24b26c98586 100644 --- a/.github/scripts/pull-request-dashboard/state.py +++ b/.github/scripts/pull-request-dashboard/state.py @@ -48,7 +48,7 @@ STATUS_COMMENT_ROLLOUT_STATE_VERSION = 2 # Rendered status-comment behavior. Increment when existing comments need to # adopt a change; hourly runs durably roll it out to all open PRs. -STATUS_COMMENT_REVISION = 17 +STATUS_COMMENT_REVISION = 18 INITIAL_BACKFILL_COMPLETE_KEY = "initial_backfill_complete" _state_dir: Path | None = None diff --git a/.github/scripts/pull-request-dashboard/test_pr_status_comment.py b/.github/scripts/pull-request-dashboard/test_pr_status_comment.py index 47d6b65df314..a5f521018a1e 100644 --- a/.github/scripts/pull-request-dashboard/test_pr_status_comment.py +++ b/.github/scripts/pull-request-dashboard/test_pr_status_comment.py @@ -373,8 +373,8 @@ def test_waiting_on_reviewers_names_maintainer_owned_blocker(self) -> None: self.assertIn("**Waiting on reviewers** · refreshed ", body) self.assertIn("Review the latest changes.", body) self.assertIn( - "**Maintainer action required:** 1 required check needs a maintainer " - "to approve or otherwise unblock its workflow.", + "**Write access required:** 1 required check needs action from someone " + "with write access to this repository.", body, ) self.assertNotIn("status check is failing", body) @@ -393,8 +393,8 @@ def test_waiting_on_maintainers_leads_with_permission_action(self) -> None: self.assertIn("Approve or otherwise unblock the required workflow checks.", body) self.assertIn( - "**Maintainer action required:** 2 required checks need a maintainer " - "to approve or otherwise unblock their workflows.", + "**Write access required:** 2 required checks need action from someone " + "with write access to this repository.", body, ) self.assertNotIn("Merge when ready.", body) @@ -411,7 +411,7 @@ def test_mixed_failure_and_permission_action_names_both_blockers(self) -> None: ) self.assertIn("Investigate required status check failures.", body) - self.assertIn("**Maintainer action required:**", body) + self.assertIn("**Write access required:**", body) def test_waiting_on_author_names_merge_conflicts(self) -> None: body = pr_status_comment.render_status_comment( diff --git a/.github/scripts/pull-request-dashboard/test_render.py b/.github/scripts/pull-request-dashboard/test_render.py index f553c053a77f..33fb4d558b3f 100644 --- a/.github/scripts/pull-request-dashboard/test_render.py +++ b/.github/scripts/pull-request-dashboard/test_render.py @@ -16,9 +16,9 @@ class RenderTest(unittest.TestCase): - def test_maintainer_owned_check_action_is_a_ci_blocker(self) -> None: + def test_permission_owned_check_action_has_a_distinct_ci_icon(self) -> None: self.assertEqual( - "❌", + "🔐", ci_cell(dashboard_facts( ci_failing_count=0, ci_maintainer_action_required_count=1, @@ -26,6 +26,25 @@ def test_maintainer_owned_check_action_is_a_ci_blocker(self) -> None: )), ) + def test_mixed_failure_and_permission_action_shows_both_icons(self) -> None: + self.assertEqual( + "❌ 🔐", + ci_cell(dashboard_facts( + ci_failing_count=1, + ci_maintainer_action_required_count=1, + ci_pending_count=0, + )), + ) + + def test_ci_legend_explains_write_access_icon(self) -> None: + markdown = render_pr_tables([], ()) + + self.assertIn( + "CI column: ✅ passing · ⏳ running · ❌ failing · " + "🔐 write access required.", + markdown, + ) + def test_reviewer_legend_includes_top_level_feedback(self) -> None: markdown = render_pr_tables([], ()) From eda37760cad1b90d61adfbee0775d231fcd88d92 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Wed, 2 Sep 2026 13:08:19 -0700 Subject: [PATCH 4/9] Address permission check review findings Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9dc6c323-cc68-4ec7-a881-b441bc269583 --- .github/scripts/pull-request-dashboard/RATIONALE.md | 11 ++++++++++- .github/scripts/pull-request-dashboard/github_cli.py | 3 +-- .../scripts/pull-request-dashboard/test_github_cli.py | 8 +------- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/.github/scripts/pull-request-dashboard/RATIONALE.md b/.github/scripts/pull-request-dashboard/RATIONALE.md index c8f61f96ef63..8a599ec3fd2f 100644 --- a/.github/scripts/pull-request-dashboard/RATIONALE.md +++ b/.github/scripts/pull-request-dashboard/RATIONALE.md @@ -280,7 +280,9 @@ the implementation understandable and operationally cheap. CI failure, including when review feedback also needs author action. Repository-configured `non_blocking_check_patterns` identify failed optional checks in a note alongside this action, without changing required-check facts - or routing. + or routing. An optional check in `ACTION_REQUIRED` is not included in that + failure-only note: it does not block merge and has neither failed nor been + cancelled. - A merge conflict does not decide who should act. Discussion, CI, and approval routing still identify the owner, while the conflict remains visible as a separate merge blocker. This lets maintainers handle routine conflicts, such @@ -301,6 +303,13 @@ the implementation understandable and operationally cheap. author is never held, because a failing check or new author-owned discussion is evidence the gates cannot undo. Unavailable check results hold the handoff for the same reason a pending one does, and resolve on a later run. +- `ACTION_REQUIRED` is the exception to that hold. It is a final, reported + result that still blocks merge because someone with repository write access + must approve or otherwise unblock the workflow. It remains unsettled and + appears as 🔐 in the CI column, but it does not hold routing with the author: + the PR routes to reviewers while approvals are outstanding, then to + maintainers once it has enough approvals. A genuine required-check failure, + including one alongside `ACTION_REQUIRED`, still routes to the author. - A held PR is presented as waiting on its author rather than on the robot it is waiting for, so a separate route would add a section that nobody is expected to act on. What it waits for is named in the columns instead: the CI diff --git a/.github/scripts/pull-request-dashboard/github_cli.py b/.github/scripts/pull-request-dashboard/github_cli.py index 86c415d80f84..1acfb6b4188f 100644 --- a/.github/scripts/pull-request-dashboard/github_cli.py +++ b/.github/scripts/pull-request-dashboard/github_cli.py @@ -491,8 +491,7 @@ def gh_pr_check_rollup( check for check, is_required in checks if not is_required - and check.get("bucket") - in ("fail", "cancel", "maintainer_action_required") + and check.get("bucket") in ("fail", "cancel") ], "code_scanning": [ check diff --git a/.github/scripts/pull-request-dashboard/test_github_cli.py b/.github/scripts/pull-request-dashboard/test_github_cli.py index b96df7cde414..d10167fa3642 100644 --- a/.github/scripts/pull-request-dashboard/test_github_cli.py +++ b/.github/scripts/pull-request-dashboard/test_github_cli.py @@ -161,13 +161,7 @@ def test_optional_action_required_check_remains_non_blocking( assert rollup is not None self.assertEqual([], rollup["required"]) - self.assertEqual( - ["maintainer_action_required"], - [ - check["bucket"] - for check in rollup["non_blocking_failures"] - ], - ) + self.assertEqual([], rollup["non_blocking_failures"]) @patch("github_cli.run_gh_json") def test_pr_view_fetches_body_for_routing_freshness(self, run_json) -> None: From 7e79359cf34ad218492f49ca9d27ba98dac9f062 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Wed, 2 Sep 2026 13:24:43 -0700 Subject: [PATCH 5/9] Narrow workflow approval detection Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9dc6c323-cc68-4ec7-a881-b441bc269583 --- .../pull-request-dashboard/RATIONALE.md | 17 +++--- .../pull-request-dashboard/github_cli.py | 27 ++++++++-- .../pull_request_evaluation.py | 2 +- .../pull-request-dashboard/test_dashboard.py | 53 ++++++++++++++++++- .../pull-request-dashboard/test_github_cli.py | 39 +++++++++++++- .../test_pr_status_comment.py | 16 ++++++ 6 files changed, 137 insertions(+), 17 deletions(-) diff --git a/.github/scripts/pull-request-dashboard/RATIONALE.md b/.github/scripts/pull-request-dashboard/RATIONALE.md index 8a599ec3fd2f..7075c26ab053 100644 --- a/.github/scripts/pull-request-dashboard/RATIONALE.md +++ b/.github/scripts/pull-request-dashboard/RATIONALE.md @@ -303,13 +303,16 @@ the implementation understandable and operationally cheap. author is never held, because a failing check or new author-owned discussion is evidence the gates cannot undo. Unavailable check results hold the handoff for the same reason a pending one does, and resolve on a later run. -- `ACTION_REQUIRED` is the exception to that hold. It is a final, reported - result that still blocks merge because someone with repository write access - must approve or otherwise unblock the workflow. It remains unsettled and - appears as 🔐 in the CI column, but it does not hold routing with the author: - the PR routes to reviewers while approvals are outstanding, then to - maintainers once it has enough approvals. A genuine required-check failure, - including one alongside `ACTION_REQUIRED`, still routes to the author. +- An `ACTION_REQUIRED` check from the GitHub Actions app with an attached + workflow run is the exception to that hold. That metadata identifies a + workflow approval: a final, reported result that still blocks merge because + someone with repository write access must unblock the workflow. It remains + unsettled and appears as 🔐 in the CI column, but it does not hold routing + with the author. The PR routes to reviewers while approvals are outstanding, + then to maintainers once it has enough approvals. `ACTION_REQUIRED` from + another app, or without a workflow run, has unknown ownership and routes + conservatively as a failure. A genuine or unknown-owner required-check + failure alongside a workflow approval also routes to the author. - A held PR is presented as waiting on its author rather than on the robot it is waiting for, so a separate route would add a section that nobody is expected to act on. What it waits for is named in the columns instead: the CI diff --git a/.github/scripts/pull-request-dashboard/github_cli.py b/.github/scripts/pull-request-dashboard/github_cli.py index 1acfb6b4188f..f69b3cdfda28 100644 --- a/.github/scripts/pull-request-dashboard/github_cli.py +++ b/.github/scripts/pull-request-dashboard/github_cli.py @@ -14,6 +14,7 @@ DEFAULT_OWNER = "open-telemetry" COPILOT_REVIEWER_BOT_ID = "BOT_kgDOCnlnWA" CODE_SCANNING_APP_ID = 57789 # github-advanced-security +GITHUB_ACTIONS_APP_ID = 15368 REQUEST_COPILOT_REVIEW_MUTATION = """ @@ -362,13 +363,23 @@ def fetch_pr_reviews(owner: str, repo_name: str, number: int) -> list[dict[str, """ -def check_bucket(state: str) -> str: +def check_bucket( + state: str, + *, + integration_id: int | None = None, + workflow_run_id: int | None = None, +) -> str: if state == "SUCCESS": return "pass" if state in ("SKIPPED", "NEUTRAL"): return "skipping" if state == "ACTION_REQUIRED": - return "maintainer_action_required" + if ( + integration_id == GITHUB_ACTIONS_APP_ID + and workflow_run_id is not None + ): + return "maintainer_action_required" + return "action_required" if state in ("ERROR", "FAILURE", "TIMED_OUT", "STARTUP_FAILURE"): return "fail" if state == "CANCELLED": @@ -382,6 +393,8 @@ def normalize_check(node: dict[str, Any]) -> dict[str, Any]: app = suite.get("app") or {} workflow_run = suite.get("workflowRun") or {} workflow = workflow_run.get("workflow") or {} + integration_id = None if is_status else app.get("databaseId") + workflow_run_id = None if is_status else workflow_run.get("databaseId") state = ( node.get("state") if is_status @@ -390,15 +403,19 @@ def normalize_check(node: dict[str, Any]) -> dict[str, Any]: return { "name": (node.get("context") if is_status else node.get("name")) or "", "state": state, - "bucket": check_bucket(state), + "bucket": check_bucket( + state, + integration_id=integration_id, + workflow_run_id=workflow_run_id, + ), "workflow": workflow.get("name") or "", - "workflow_run_id": workflow_run.get("databaseId"), + "workflow_run_id": workflow_run_id, "description": node.get("description") or "", "link": (node.get("targetUrl") if is_status else node.get("detailsUrl")) or "", "started_at": (node.get("createdAt") if is_status else node.get("startedAt")) or "", "completed_at": (node.get("createdAt") if is_status else node.get("completedAt")) or "", "check_run_id": None if is_status else check_run_id(node["url"]), - "integration_id": None if is_status else app.get("databaseId"), + "integration_id": integration_id, "status_context": is_status, } diff --git a/.github/scripts/pull-request-dashboard/pull_request_evaluation.py b/.github/scripts/pull-request-dashboard/pull_request_evaluation.py index 44487c80d92c..5aef4fdb4b01 100644 --- a/.github/scripts/pull-request-dashboard/pull_request_evaluation.py +++ b/.github/scripts/pull-request-dashboard/pull_request_evaluation.py @@ -163,7 +163,7 @@ def _compute_facts( failing = [ check for check in checks or () - if check.bucket in ("fail", "cancel") + if check.bucket in ("fail", "cancel", "action_required") ] pending = [ check diff --git a/.github/scripts/pull-request-dashboard/test_dashboard.py b/.github/scripts/pull-request-dashboard/test_dashboard.py index c226435a49e1..c4084b275205 100644 --- a/.github/scripts/pull-request-dashboard/test_dashboard.py +++ b/.github/scripts/pull-request-dashboard/test_dashboard.py @@ -1822,10 +1822,10 @@ def test_required_check_buckets_control_ci_facts(self) -> None: ("TIMED_OUT", "fail", 1, 0, 0), ( "ACTION_REQUIRED", - "maintainer_action_required", - 0, + "action_required", 1, 0, + 0, ), ("STARTUP_FAILURE", "fail", 1, 0, 0), ("CANCELLED", "cancel", 1, 0, 0), @@ -1865,6 +1865,55 @@ def test_required_check_buckets_control_ci_facts(self) -> None: facts.non_blocking_check_failures, ) + def test_workflow_approval_is_distinct_from_generic_action_required( + self, + ) -> None: + facts = evaluation_facts( + { + "pr": { + "createdAt": "2026-07-14T01:00:00Z", + "author": {"login": "author"}, + "mergeStateStatus": "CLEAN", + "mergeable": "MERGEABLE", + }, + "checks": [ + {"state": "ACTION_REQUIRED", "bucket": "action_required"}, + { + "state": "ACTION_REQUIRED", + "bucket": "maintainer_action_required", + }, + ], + }, + "author", + [], + ) + + self.assertEqual(1, facts.ci_failing_count) + self.assertEqual(1, facts.ci_maintainer_action_required_count) + + @patch("pull_request_evaluation.fetch_pull_request_source") + def test_generic_action_required_routes_to_author( + self, + fetch_source: Mock, + ) -> None: + fetch_source.return_value = pull_request_source( + checks=(check_source( + state="ACTION_REQUIRED", + bucket="action_required", + ),), + ) + + result = evaluate_pr({"number": 7}) + + self.assertIsInstance(result, EvaluationSuccess) + assert isinstance(result, EvaluationSuccess) + self.assertEqual(DashboardRoute.AUTHOR, result.route) + self.assertEqual(1, result.facts.ci_failing_count) + self.assertEqual( + 0, + result.facts.ci_maintainer_action_required_count, + ) + def test_override_command_does_not_clear_required_check_failures(self) -> None: facts = evaluation_facts( { diff --git a/.github/scripts/pull-request-dashboard/test_github_cli.py b/.github/scripts/pull-request-dashboard/test_github_cli.py index d10167fa3642..a23e5a27a14a 100644 --- a/.github/scripts/pull-request-dashboard/test_github_cli.py +++ b/.github/scripts/pull-request-dashboard/test_github_cli.py @@ -19,6 +19,7 @@ is_retryable_gh_error, list_open_prs, merge_code_scanning_checks, + normalize_check, request_copilot_review, required_check_contexts, required_code_scanning_checks, @@ -131,14 +132,48 @@ def test_follows_pagination(self) -> None: class GithubCliTest(unittest.TestCase): - def test_action_required_has_a_maintainer_owned_bucket(self) -> None: + def test_generic_action_required_keeps_unknown_ownership(self) -> None: self.assertEqual( - "maintainer_action_required", + "action_required", check_bucket("ACTION_REQUIRED"), ) self.assertEqual("fail", check_bucket("FAILURE")) self.assertEqual("pending", check_bucket("IN_PROGRESS")) + def test_actions_workflow_approval_has_a_maintainer_owned_bucket( + self, + ) -> None: + check = normalize_check({ + "__typename": "CheckRun", + "name": "build", + "status": "COMPLETED", + "conclusion": "ACTION_REQUIRED", + "url": "https://github.com/open-telemetry/example/runs/1", + "checkSuite": { + "app": {"databaseId": 15368}, + "workflowRun": { + "databaseId": 101, + "workflow": {"name": "CI"}, + }, + }, + }) + + self.assertEqual("maintainer_action_required", check["bucket"]) + + def test_actions_action_required_without_a_workflow_run_is_generic( + self, + ) -> None: + check = normalize_check({ + "__typename": "CheckRun", + "name": "custom check", + "status": "COMPLETED", + "conclusion": "ACTION_REQUIRED", + "url": "https://github.com/open-telemetry/example/runs/1", + "checkSuite": {"app": {"databaseId": 15368}}, + }) + + self.assertEqual("action_required", check["bucket"]) + @patch("github_cli.gh_graphql") def test_optional_action_required_check_remains_non_blocking( self, diff --git a/.github/scripts/pull-request-dashboard/test_pr_status_comment.py b/.github/scripts/pull-request-dashboard/test_pr_status_comment.py index a5f521018a1e..b65181378eb6 100644 --- a/.github/scripts/pull-request-dashboard/test_pr_status_comment.py +++ b/.github/scripts/pull-request-dashboard/test_pr_status_comment.py @@ -359,6 +359,22 @@ def test_waiting_on_author_names_required_ci_failure(self) -> None: self.assertNotIn("### Review feedback", body) self.assertNotIn(pr_status_comment.RESPONSE_EXAMPLES, body) + def test_generic_action_required_renders_as_an_author_owned_failure( + self, + ) -> None: + body = pr_status_comment.render_status_comment( + self.pr(), + status_result( + DashboardRoute.AUTHOR, + ci_failing_count=1, + ci_maintainer_action_required_count=0, + ci_pending_count=0, + ), + ) + + self.assertIn("Investigate required status check failures.", body) + self.assertNotIn("Write access required", body) + def test_waiting_on_reviewers_names_maintainer_owned_blocker(self) -> None: body = pr_status_comment.render_status_comment( self.pr(), From 5f0c966cbf2ddea71ab66479b5ab024464cb2fc9 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Wed, 2 Sep 2026 13:26:46 -0700 Subject: [PATCH 6/9] Label permission checks as workflow actions Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9dc6c323-cc68-4ec7-a881-b441bc269583 --- .../scripts/pull-request-dashboard/pr_status_comment.py | 6 +++--- .github/scripts/pull-request-dashboard/render.py | 2 +- .github/scripts/pull-request-dashboard/state.py | 2 +- .../pull-request-dashboard/test_pr_status_comment.py | 8 ++++---- .github/scripts/pull-request-dashboard/test_render.py | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/.github/scripts/pull-request-dashboard/pr_status_comment.py b/.github/scripts/pull-request-dashboard/pr_status_comment.py index f38573ae84ae..8b77ed8ef827 100644 --- a/.github/scripts/pull-request-dashboard/pr_status_comment.py +++ b/.github/scripts/pull-request-dashboard/pr_status_comment.py @@ -216,7 +216,7 @@ def author_body( return [fallback_next_step] -def write_access_required_summary(count: int) -> str: +def workflow_action_required_summary(count: int) -> str: if count == 1: return ( "1 required check needs action from someone with write access " @@ -343,8 +343,8 @@ def render_status_comment( body.extend([ "", ( - "**Write access required:** " - + write_access_required_summary( + "**Workflow action required:** " + + workflow_action_required_summary( maintainer_action_required_count ) ), diff --git a/.github/scripts/pull-request-dashboard/render.py b/.github/scripts/pull-request-dashboard/render.py index 95de0503e92d..2f356923aa66 100644 --- a/.github/scripts/pull-request-dashboard/render.py +++ b/.github/scripts/pull-request-dashboard/render.py @@ -251,7 +251,7 @@ def render_pr_tables( ) ci_note = ( "CI column: ✅ passing · ⏳ running · ❌ failing · " - "🔐 write access required." + "🔐 workflow action required." ) out: list[str] = [ "> [!NOTE]", diff --git a/.github/scripts/pull-request-dashboard/state.py b/.github/scripts/pull-request-dashboard/state.py index b24b26c98586..28381a0b3c79 100644 --- a/.github/scripts/pull-request-dashboard/state.py +++ b/.github/scripts/pull-request-dashboard/state.py @@ -48,7 +48,7 @@ STATUS_COMMENT_ROLLOUT_STATE_VERSION = 2 # Rendered status-comment behavior. Increment when existing comments need to # adopt a change; hourly runs durably roll it out to all open PRs. -STATUS_COMMENT_REVISION = 18 +STATUS_COMMENT_REVISION = 19 INITIAL_BACKFILL_COMPLETE_KEY = "initial_backfill_complete" _state_dir: Path | None = None diff --git a/.github/scripts/pull-request-dashboard/test_pr_status_comment.py b/.github/scripts/pull-request-dashboard/test_pr_status_comment.py index b65181378eb6..f20855515a09 100644 --- a/.github/scripts/pull-request-dashboard/test_pr_status_comment.py +++ b/.github/scripts/pull-request-dashboard/test_pr_status_comment.py @@ -373,7 +373,7 @@ def test_generic_action_required_renders_as_an_author_owned_failure( ) self.assertIn("Investigate required status check failures.", body) - self.assertNotIn("Write access required", body) + self.assertNotIn("Workflow action required", body) def test_waiting_on_reviewers_names_maintainer_owned_blocker(self) -> None: body = pr_status_comment.render_status_comment( @@ -389,7 +389,7 @@ def test_waiting_on_reviewers_names_maintainer_owned_blocker(self) -> None: self.assertIn("**Waiting on reviewers** · refreshed ", body) self.assertIn("Review the latest changes.", body) self.assertIn( - "**Write access required:** 1 required check needs action from someone " + "**Workflow action required:** 1 required check needs action from someone " "with write access to this repository.", body, ) @@ -409,7 +409,7 @@ def test_waiting_on_maintainers_leads_with_permission_action(self) -> None: self.assertIn("Approve or otherwise unblock the required workflow checks.", body) self.assertIn( - "**Write access required:** 2 required checks need action from someone " + "**Workflow action required:** 2 required checks need action from someone " "with write access to this repository.", body, ) @@ -427,7 +427,7 @@ def test_mixed_failure_and_permission_action_names_both_blockers(self) -> None: ) self.assertIn("Investigate required status check failures.", body) - self.assertIn("**Write access required:**", body) + self.assertIn("**Workflow action required:**", body) def test_waiting_on_author_names_merge_conflicts(self) -> None: body = pr_status_comment.render_status_comment( diff --git a/.github/scripts/pull-request-dashboard/test_render.py b/.github/scripts/pull-request-dashboard/test_render.py index 33fb4d558b3f..846914b7d77e 100644 --- a/.github/scripts/pull-request-dashboard/test_render.py +++ b/.github/scripts/pull-request-dashboard/test_render.py @@ -41,7 +41,7 @@ def test_ci_legend_explains_write_access_icon(self) -> None: self.assertIn( "CI column: ✅ passing · ⏳ running · ❌ failing · " - "🔐 write access required.", + "🔐 workflow action required.", markdown, ) From 48f8070f38dbff13e1797f783b24639fa0c289e4 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Wed, 2 Sep 2026 13:53:25 -0700 Subject: [PATCH 7/9] Address Copilot review comment: reject action-required review requests 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> --- .../pull-request-dashboard/copilot_review.py | 2 +- .../test_copilot_review.py | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/.github/scripts/pull-request-dashboard/copilot_review.py b/.github/scripts/pull-request-dashboard/copilot_review.py index 57056b40f4ed..ea3285d404ef 100644 --- a/.github/scripts/pull-request-dashboard/copilot_review.py +++ b/.github/scripts/pull-request-dashboard/copilot_review.py @@ -222,7 +222,7 @@ def stale_request_reason( failing = [ check for check in checks - if check.bucket in ("fail", "cancel") + if check.bucket in ("fail", "cancel", "action_required") ] if failing: return f"required checks are failing: {named_checks(failing)}" diff --git a/.github/scripts/pull-request-dashboard/test_copilot_review.py b/.github/scripts/pull-request-dashboard/test_copilot_review.py index 3874c187d30e..50551df5719e 100644 --- a/.github/scripts/pull-request-dashboard/test_copilot_review.py +++ b/.github/scripts/pull-request-dashboard/test_copilot_review.py @@ -986,6 +986,23 @@ def test_pending_required_checks_do_not_make_a_request_stale(self) -> None: ), ) + def test_request_recorded_while_pending_is_stale_after_action_required( + self, + ) -> None: + # The unchanged fingerprint models a request recorded while this check + # was pending because check results are not part of that fingerprint. + self.assertEqual( + "required checks are failing: build", + self.reason( + raw={ + "checks": [ + {"name": "build", "bucket": "action_required"}, + {"name": "lint", "bucket": "pass"}, + ], + }, + ), + ) + def test_summarizes_long_lists_of_failing_checks(self) -> None: self.assertEqual( "required checks are failing: a, b, c and 2 more", From 2f377ec3015f04a32c7038058f1c91b0046a964e Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Wed, 2 Sep 2026 14:18:19 -0700 Subject: [PATCH 8/9] Address review findings: describe the version 14 state regeneration accurately 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 257477f7646 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> --- .github/scripts/pull-request-dashboard/CONTEXT.md | 2 +- .github/scripts/pull-request-dashboard/test_state.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/scripts/pull-request-dashboard/CONTEXT.md b/.github/scripts/pull-request-dashboard/CONTEXT.md index c341a33c0f8a..8cac8583d296 100644 --- a/.github/scripts/pull-request-dashboard/CONTEXT.md +++ b/.github/scripts/pull-request-dashboard/CONTEXT.md @@ -30,7 +30,7 @@ diagnostics keep typed classification results and freeze only the source discussion records. `state.py` owns the JSON boundary. Its dashboard facts, stored-result, and state -codecs translate the immutable contracts to the version 13 +codecs translate the immutable contracts to the version 14 `dashboard-state.json` shape. Malformed pull request entries are discarded individually, so one bad entry does not prevent valid entries from loading. diff --git a/.github/scripts/pull-request-dashboard/test_state.py b/.github/scripts/pull-request-dashboard/test_state.py index f663c68893a4..9a33426e2b73 100644 --- a/.github/scripts/pull-request-dashboard/test_state.py +++ b/.github/scripts/pull-request-dashboard/test_state.py @@ -420,7 +420,7 @@ def test_malformed_persisted_results_are_rejected_individually(self) -> None: "warning: ignoring malformed dashboard result" )) - def test_version_eleven_dashboard_state_migrates_to_current_shape(self) -> None: + def test_legacy_dashboard_state_payload_reencodes_to_current_shape(self) -> None: persisted = { "version": 11, "initial_backfill_complete": True, From b9c50a3ebe82f73aa8deb2bc4df23e1f382af2a8 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Wed, 2 Sep 2026 14:35:49 -0700 Subject: [PATCH 9/9] Show pending workflow approval checks Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../scripts/pull-request-dashboard/render.py | 2 ++ .../pull-request-dashboard/test_dashboard.py | 27 +++++++++++++++++-- .../test_pr_status_comment.py | 24 +++++++++++++++++ .../pull-request-dashboard/test_render.py | 10 +++++++ 4 files changed, 61 insertions(+), 2 deletions(-) diff --git a/.github/scripts/pull-request-dashboard/render.py b/.github/scripts/pull-request-dashboard/render.py index 2f356923aa66..3c4f3f92556c 100644 --- a/.github/scripts/pull-request-dashboard/render.py +++ b/.github/scripts/pull-request-dashboard/render.py @@ -105,6 +105,8 @@ def ci_cell(facts: DashboardFacts) -> str: return "❌ 🔐" if failing: return "❌" + if (facts.ci_pending_count or 0) > 0 and write_access_required: + return "⏳ 🔐" if write_access_required: return "🔐" if (facts.ci_pending_count or 0) > 0: diff --git a/.github/scripts/pull-request-dashboard/test_dashboard.py b/.github/scripts/pull-request-dashboard/test_dashboard.py index c4084b275205..dc959f4e7939 100644 --- a/.github/scripts/pull-request-dashboard/test_dashboard.py +++ b/.github/scripts/pull-request-dashboard/test_dashboard.py @@ -80,6 +80,7 @@ evaluate_pull_request, ) from pull_request_activity import PullRequestActivity +from render import render_pr_tables from reviewer_state import ReviewerInput, prepare_reviewers from routing_decision import resolve_routing @@ -992,12 +993,18 @@ def test_normal_routing_flows_through_evaluation( @patch("routing_decision.utc_now") @patch("pull_request_evaluation.fetch_pull_request_source") - def test_running_required_check_keeps_integrated_route_held( + def test_running_check_and_workflow_approval_keep_integrated_route_held( self, fetch_raw: Mock, utc_now: Mock ) -> None: utc_now.return_value = datetime(2026, 8, 16, 12, 0, tzinfo=timezone.utc) fetch_raw.return_value = self.raw_pr( - checks=[{"name": "required", "bucket": "pending"}] + checks=[ + {"name": "required", "bucket": "pending"}, + { + "name": "workflow approval", + "bucket": "maintainer_action_required", + }, + ] ) classifier = FakeClassificationOperation() @@ -1022,6 +1029,22 @@ def test_running_required_check_keeps_integrated_route_held( self.assertEqual( "2026-08-16T12:00:00+00:00", result.facts.route_held_since ) + self.assertEqual(1, result.facts.ci_pending_count) + self.assertEqual(1, result.facts.ci_maintainer_action_required_count) + markdown = render_pr_tables( + [{ + "number": 7, + "title": "Pull request", + "author": {"login": "author"}, + "isDraft": False, + }], + (stored_dashboard_result( + 7, + route=result.route, + facts=result.facts, + ),), + ) + self.assertIn("| ⏳ 🔐 |", markdown) self.assertEqual(len(classifier.requests), 1) @patch("pull_request_evaluation.fetch_pull_request_source") diff --git a/.github/scripts/pull-request-dashboard/test_pr_status_comment.py b/.github/scripts/pull-request-dashboard/test_pr_status_comment.py index f20855515a09..a965c15f8169 100644 --- a/.github/scripts/pull-request-dashboard/test_pr_status_comment.py +++ b/.github/scripts/pull-request-dashboard/test_pr_status_comment.py @@ -395,6 +395,30 @@ def test_waiting_on_reviewers_names_maintainer_owned_blocker(self) -> None: ) self.assertNotIn("status check is failing", body) + def test_pending_check_and_workflow_action_name_both_blockers(self) -> None: + body = pr_status_comment.render_status_comment( + self.pr(), + status_result( + DashboardRoute.AUTHOR, + ci_failing_count=0, + ci_maintainer_action_required_count=1, + ci_pending_count=1, + required_checks_settled=False, + route_held_for_gates=True, + ), + ) + + self.assertIn( + "Wait for the required status checks to report; this pull request " + "moves to reviewers once the results are clean.", + body, + ) + self.assertIn( + "**Workflow action required:** 1 required check needs action from someone " + "with write access to this repository.", + body, + ) + def test_waiting_on_maintainers_leads_with_permission_action(self) -> None: body = pr_status_comment.render_status_comment( self.pr(), diff --git a/.github/scripts/pull-request-dashboard/test_render.py b/.github/scripts/pull-request-dashboard/test_render.py index 846914b7d77e..430c1afd25e0 100644 --- a/.github/scripts/pull-request-dashboard/test_render.py +++ b/.github/scripts/pull-request-dashboard/test_render.py @@ -26,6 +26,16 @@ def test_permission_owned_check_action_has_a_distinct_ci_icon(self) -> None: )), ) + def test_pending_check_and_permission_action_show_both_icons(self) -> None: + self.assertEqual( + "⏳ 🔐", + ci_cell(dashboard_facts( + ci_failing_count=0, + ci_maintainer_action_required_count=1, + ci_pending_count=1, + )), + ) + def test_mixed_failure_and_permission_action_shows_both_icons(self) -> None: self.assertEqual( "❌ 🔐",