diff --git a/.github/scripts/pull-request-dashboard/CONTEXT.md b/.github/scripts/pull-request-dashboard/CONTEXT.md index c341a33c0f8..1fd1b2e867d 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 15 `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/RATIONALE.md b/.github/scripts/pull-request-dashboard/RATIONALE.md index c8f61f96ef6..757f524c0a7 100644 --- a/.github/scripts/pull-request-dashboard/RATIONALE.md +++ b/.github/scripts/pull-request-dashboard/RATIONALE.md @@ -336,11 +336,17 @@ the implementation understandable and operationally cheap. with reviewers and is released to maintainers keeps its wait, because it never left the people who owe it a response, and restarting there would present an approval a week old as a merge request that just arrived. -- Maintenance-bot PRs retain maintainer-oriented routing because the bot cannot - respond to a dashboard action. Pending required checks affect the CI column - but never route one of these PRs to its author: a bot PR whose handoff is - held waits on reviewers instead. Merge conflicts remain visible without - overriding that routing. +- PRs from unattended automation never route to their author because the + automation cannot respond to a dashboard action. Copilot-delegated PRs are + different. The dashboard recovers a human assignee or the first commit's + human committer as the effective author, so that person can receive the + author route. Pending required checks affect the CI column, and a bot PR + whose handoff is held waits on reviewers instead. Known maintenance bots use + a one-approval threshold; other automation uses the repository's configured + threshold. Compatible cached state derives a missing author-capability fact + from the effective author identity and discards a stale author-routed result + for reevaluation, while an explicit stored value wins. Merge conflicts remain + visible without overriding that routing. - A hold has a time limit, and past it the PR routes anyway. Every gate waits on something outside the dashboard, and each one has been seen never to arrive: a required check with no check run on the head, a Copilot review GitHub never diff --git a/.github/scripts/pull-request-dashboard/dashboard.py b/.github/scripts/pull-request-dashboard/dashboard.py index b42dd8d8f7f..ebbf3a60582 100644 --- a/.github/scripts/pull-request-dashboard/dashboard.py +++ b/.github/scripts/pull-request-dashboard/dashboard.py @@ -104,6 +104,8 @@ assignees list[str] PR assignees. is_maintenance_bot bool PR is authored by a maintenance bot. + author_can_act bool Effective author can respond + to author-routed work. is_draft bool approval_count int Current unique APPROVED reviews from approver-team members, diff --git a/.github/scripts/pull-request-dashboard/dashboard_contracts.py b/.github/scripts/pull-request-dashboard/dashboard_contracts.py index 566d7de033f..7fe5174ec25 100644 --- a/.github/scripts/pull-request-dashboard/dashboard_contracts.py +++ b/.github/scripts/pull-request-dashboard/dashboard_contracts.py @@ -92,6 +92,7 @@ class DashboardFacts: copilot_review_stale: bool = False copilot_review_needed: bool = False is_maintenance_bot: bool = False + author_can_act: bool = True is_draft: bool = False approval_count: int = 0 conflicts: str = "unknown" diff --git a/.github/scripts/pull-request-dashboard/pull_request_evaluation.py b/.github/scripts/pull-request-dashboard/pull_request_evaluation.py index 476ad1d3cde..2d0cce54674 100644 --- a/.github/scripts/pull-request-dashboard/pull_request_evaluation.py +++ b/.github/scripts/pull-request-dashboard/pull_request_evaluation.py @@ -53,6 +53,7 @@ ) from github_cli import TransientGhError from pull_request_source import ( + Actor, IssueComment, PullRequestSource, fetch_pull_request_source, @@ -76,16 +77,46 @@ routing_failure_facts, ) from routing_snapshot import build_routing_snapshot -from utils import format_ts, parse_ts +from utils import ( + format_ts, + is_unattended_author_login, + normalize_author_identity, + parse_ts, +) + + +# Copilot appears under two slugs: `gh pr view`'s `author` field reports +# `app/copilot-swe-agent`, while the Pulls/commits endpoint's `committer.login` +# field can report the bare `copilot` slug. Either slug can name Copilot as the +# author, so the author set carries both while the committer set carries only +# the bare slug. These sets hold the identities `normalize_author_identity` +# returns, without the `app/` prefix or the `[bot]` suffix. Do not treat either +# slug as the human author behind a Copilot-authored PR. +_COPILOT_COMMITTER_IDENTITIES = {"copilot"} +_COPILOT_PR_AUTHOR_IDENTITIES = {"copilot-swe-agent", "copilot"} +_MAINTENANCE_APP_IDENTITIES = {"dependabot", "otelbot", "renovate"} -# Copilot appears in two API shapes: `gh pr view`'s `author` field uses the -# `app/` form, while the Pulls/commits endpoint's `committer.login` -# field can return the bare `copilot` slug. Do not treat either form as the -# human author behind a Copilot-authored PR. -_COPILOT_COMMITTER_LOGINS = {"copilot"} -_COPILOT_PR_AUTHORS = {"app/copilot-swe-agent", "copilot"} -_MAINTENANCE_BOT_PR_AUTHORS = {"app/otelbot", "app/renovate"} +def _is_maintenance_bot_author(login: str) -> bool: + normalized_login = (login or "").strip().casefold() + identity = normalize_author_identity(normalized_login) + return identity == "opentelemetrybot" or ( + identity in _MAINTENANCE_APP_IDENTITIES + and ( + normalized_login.startswith("app/") + or normalized_login.endswith("[bot]") + ) + ) + + +def _author_can_act(api_author: Actor, effective_author: str) -> bool: + if is_unattended_author_login(effective_author): + return False + return ( + not api_author.is_bot + or normalize_author_identity(api_author.login) + != normalize_author_identity(effective_author) + ) @dataclass(frozen=True) @@ -116,12 +147,11 @@ def _human_author_for_copilot_pr(source: PullRequestSource) -> str: for assignee in source.pull_request.assignees ] for login in assignees: - low = login.lower() + identity = normalize_author_identity(login) if ( login - and low not in _COPILOT_PR_AUTHORS - and not low.startswith("app/") - and not low.endswith("[bot]") + and identity not in _COPILOT_PR_AUTHOR_IDENTITIES + and not is_unattended_author_login(login) ): return login @@ -132,7 +162,8 @@ def _human_author_for_copilot_pr(source: PullRequestSource) -> str: login = committer.login if ( not login - or login.lower() in _COPILOT_COMMITTER_LOGINS + or normalize_author_identity(login) in _COPILOT_COMMITTER_IDENTITIES + or is_unattended_author_login(login) or committer.is_bot or committer.is_copilot_reviewer ): @@ -142,7 +173,7 @@ def _human_author_for_copilot_pr(source: PullRequestSource) -> str: def _effective_author(source: PullRequestSource) -> str: author = source.pull_request.author.login - if author.lower() in _COPILOT_PR_AUTHORS: + if normalize_author_identity(author) in _COPILOT_PR_AUTHOR_IDENTITIES: human_author = _human_author_for_copilot_pr(source) if human_author: return human_author @@ -230,7 +261,8 @@ def _compute_facts( copilot_review_exists=copilot_review_exists, copilot_review_stale=copilot_review_stale, copilot_review_needed=copilot_review_stale or copilot_review_findings, - is_maintenance_bot=api_author.lower() in _MAINTENANCE_BOT_PR_AUTHORS, + is_maintenance_bot=_is_maintenance_bot_author(api_author), + author_can_act=_author_can_act(pr.author, author), is_draft=pr.is_draft, approval_count=prepared_reviewers.approval_count, conflicts=pr.conflicts, diff --git a/.github/scripts/pull-request-dashboard/routing_decision.py b/.github/scripts/pull-request-dashboard/routing_decision.py index b53f5ad373d..79ade178ea3 100644 --- a/.github/scripts/pull-request-dashboard/routing_decision.py +++ b/.github/scripts/pull-request-dashboard/routing_decision.py @@ -86,9 +86,9 @@ def _base_route( counts = _action_counts(pending_actions) is_maintenance_bot = facts.is_maintenance_bot approval_threshold = 1 if is_maintenance_bot else required_approvals - if (facts.ci_failing_count or 0) > 0 and not is_maintenance_bot: + if (facts.ci_failing_count or 0) > 0 and facts.author_can_act: return DashboardRoute.AUTHOR - if counts["author"] and not is_maintenance_bot: + if counts["author"] and facts.author_can_act: return DashboardRoute.AUTHOR if facts.approval_count >= approval_threshold: return DashboardRoute.MAINTAINER @@ -145,12 +145,12 @@ def _hold_route_until_gates_settle( ) -> tuple[DashboardRoute, DashboardFacts]: effective_previous_route = previous_route or DashboardRoute.AUTHOR if effective_previous_route.value not in _ROUTE_PROGRESSION or ( - facts.is_maintenance_bot + not facts.author_can_act and effective_previous_route is DashboardRoute.AUTHOR ): effective_previous_route = ( DashboardRoute.APPROVER - if facts.is_maintenance_bot + if not facts.author_can_act else DashboardRoute.AUTHOR ) gates_enabled = not bypass_gates diff --git a/.github/scripts/pull-request-dashboard/state.py b/.github/scripts/pull-request-dashboard/state.py index 98320e3dccf..bc7a3329a69 100644 --- a/.github/scripts/pull-request-dashboard/state.py +++ b/.github/scripts/pull-request-dashboard/state.py @@ -20,6 +20,7 @@ ) from github_cli import detect_repo, normalize_repo, repo_state_key import state_branch +from utils import is_unattended_author_login DASHBOARD_MARKDOWN_FILE = "pull-request-dashboard.md" @@ -35,7 +36,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 = 15 # backfill-state.json: round-robin cursor used by full dashboard refreshes. BACKFILL_STATE_VERSION = 3 # notification-state.json: pending and delivered Slack notification records. @@ -453,8 +454,9 @@ def decode_dashboard_facts(value: Any) -> DashboardFacts: raw_reviewers = [] if not isinstance(raw_reviewers, list): raise ValueError("facts.reviewers must be an array") + author = _string(value.get("author", _MISSING), "facts.author") return DashboardFacts( - author=_string(value.get("author", _MISSING), "facts.author"), + author=author, assignees=_string_tuple( value.get("assignees", _MISSING), "facts.assignees", @@ -516,6 +518,11 @@ def decode_dashboard_facts(value: Any) -> DashboardFacts: value.get("is_maintenance_bot", _MISSING), "facts.is_maintenance_bot", ), + author_can_act=_boolean( + value.get("author_can_act", _MISSING), + "facts.author_can_act", + not is_unattended_author_login(author), + ), is_draft=_boolean( value.get("is_draft", _MISSING), "facts.is_draft", @@ -636,6 +643,7 @@ def encode_dashboard_facts(facts: DashboardFacts) -> dict[str, Any]: "copilot_review_stale": facts.copilot_review_stale, "copilot_review_needed": facts.copilot_review_needed, "is_maintenance_bot": facts.is_maintenance_bot, + "author_can_act": facts.author_can_act, "is_draft": facts.is_draft, "approval_count": facts.approval_count, "conflicts": facts.conflicts, @@ -719,6 +727,11 @@ def decode_stored_result( history = {} if not isinstance(history, dict): raise ValueError("dashboard result top_level_history must be an object") + facts = decode_dashboard_facts( + value["facts"] if "facts" in value else {} + ) + if route is DashboardRoute.AUTHOR and not facts.author_can_act: + raise ValueError("dashboard result author route requires an actionable author") return StoredDashboardResult( pr_number=pr_number, pr_url=_string( @@ -726,9 +739,7 @@ def decode_stored_result( "dashboard result pr_url", ), route=route, - facts=decode_dashboard_facts( - value["facts"] if "facts" in value else {} - ), + facts=facts, top_level_history=freeze_json_object(history), ) @@ -808,7 +819,7 @@ def load_dashboard_state_cache() -> DashboardState | None: state = load_state_file( dashboard_state_path(), DASHBOARD_STATE_VERSION, - compatible_versions=(11, 12), + compatible_versions=(11, 12, 13), ) if state is None: return None diff --git a/.github/scripts/pull-request-dashboard/test_pull_request_evaluation.py b/.github/scripts/pull-request-dashboard/test_pull_request_evaluation.py index b2233e027b1..eed393d5580 100644 --- a/.github/scripts/pull-request-dashboard/test_pull_request_evaluation.py +++ b/.github/scripts/pull-request-dashboard/test_pull_request_evaluation.py @@ -23,10 +23,12 @@ from classification_test_support import FakeClassificationOperation from dashboard_test_support import ( actor, + check_source, commit_source, dashboard_facts, pull_request_metadata, pull_request_source, + review_source, stored_dashboard_result, ) from discussion_lifecycle import resolve_discussions @@ -35,7 +37,9 @@ from pull_request_evaluation import ( PullRequestEvaluationConfig, PullRequestEvaluationInput, + _author_can_act, _handoff_feedback_routes_to_author, + _is_maintenance_bot_author, evaluate_pull_request, ) @@ -129,6 +133,7 @@ def test_success_uses_the_effective_copilot_author( self.assertIsInstance(result, EvaluationSuccess) assert isinstance(result, EvaluationSuccess) self.assertEqual("human-author", result.facts.author) + self.assertTrue(result.facts.author_can_act) self.assertEqual(7, result.pr_number) self.assertEqual("Evaluation contract", result.pr_title) self.assertEqual("https://example.test/pull/7", result.pr_url) @@ -159,6 +164,114 @@ def test_copilot_bot_committer_is_not_recovered_as_the_human_author( self.assertIsInstance(result, EvaluationSuccess) assert isinstance(result, EvaluationSuccess) self.assertEqual("app/copilot-swe-agent", result.facts.author) + self.assertFalse(result.facts.author_can_act) + + @patch("pull_request_evaluation.fetch_pull_request_source") + def test_automation_user_is_not_recovered_as_a_copilot_author( + self, + fetch_source, + ) -> None: + source = raw_pr(author="app/copilot-swe-agent") + fetch_source.return_value = replace( + source, + pull_request=replace( + source.pull_request, + assignees=(actor("opentelemetrybot"),), + ), + ) + + result = evaluate_pull_request( + evaluation_config(), + PullRequestEvaluationInput(7), + FakeClassificationOperation(), + ) + + self.assertIsInstance(result, EvaluationSuccess) + assert isinstance(result, EvaluationSuccess) + self.assertEqual("app/copilot-swe-agent", result.facts.author) + self.assertFalse(result.facts.author_can_act) + + def test_maintenance_author_recognition_uses_known_github_shapes(self) -> None: + for author in ( + "opentelemetrybot", + "app/opentelemetrybot", + "opentelemetrybot[bot]", + "app/otelbot", + "otelbot[bot]", + "app/renovate", + "renovate[bot]", + "app/dependabot", + "dependabot[bot]", + ): + with self.subTest(author=author): + self.assertTrue(_is_maintenance_bot_author(author)) + + for author in ( + "human-author", + "dependabot", + "otelbot", + "renovate", + ): + with self.subTest(author=author): + self.assertFalse(_is_maintenance_bot_author(author)) + + def test_author_capability_separates_automation_from_delegation(self) -> None: + for author in ( + actor("app/dependabot"), + actor("renovate[bot]"), + actor("app/custom-automation"), + actor("opentelemetrybot"), + ): + with self.subTest(author=author.login): + self.assertFalse(_author_can_act(author, author.login)) + + self.assertTrue(_author_can_act(actor("human-author"), "human-author")) + self.assertTrue( + _author_can_act(actor("app/copilot-swe-agent"), "human-author") + ) + + @patch("pull_request_evaluation.fetch_pull_request_source") + def test_opentelemetrybot_pr_routes_to_reviewers_then_maintainers( + self, + fetch_source, + ) -> None: + source = raw_pr(author="opentelemetrybot") + approved_source = replace( + source, + reviews=(review_source(state="APPROVED"),), + ) + fetch_source.side_effect = ( + replace( + source, + checks=(check_source(state="FAILURE", bucket="fail"),), + ), + replace( + approved_source, + checks=(check_source(state="FAILURE", bucket="fail"),), + ), + ) + + awaiting_approval = evaluate_pull_request( + evaluation_config(), + PullRequestEvaluationInput(7715), + FakeClassificationOperation(), + ) + approved = evaluate_pull_request( + evaluation_config(), + PullRequestEvaluationInput(7715), + FakeClassificationOperation(), + ) + + self.assertIsInstance(awaiting_approval, EvaluationSuccess) + self.assertIsInstance(approved, EvaluationSuccess) + assert isinstance(awaiting_approval, EvaluationSuccess) + assert isinstance(approved, EvaluationSuccess) + self.assertEqual("opentelemetrybot", awaiting_approval.facts.author) + self.assertTrue(awaiting_approval.facts.is_maintenance_bot) + self.assertFalse(awaiting_approval.facts.author_can_act) + self.assertEqual(DashboardRoute.APPROVER, awaiting_approval.route) + self.assertEqual(1, approved.facts.approval_count) + self.assertEqual(DashboardRoute.MAINTAINER, approved.route) @patch( "pull_request_evaluation.resolve_discussions", diff --git a/.github/scripts/pull-request-dashboard/test_routing_decision.py b/.github/scripts/pull-request-dashboard/test_routing_decision.py index 6361ad18b4b..ef97e1188a9 100644 --- a/.github/scripts/pull-request-dashboard/test_routing_decision.py +++ b/.github/scripts/pull-request-dashboard/test_routing_decision.py @@ -248,12 +248,14 @@ def test_route_selection_preserves_discussion_and_approval_rules(self) -> None: "approver", ), ( - "maintenance bot approval threshold", + "Dependabot approval threshold", { + "author": "app/dependabot", "approval_count": 1, "ci_failing_count": 1, "ci_pending_count": 0, "is_maintenance_bot": True, + "author_can_act": False, }, {}, 2, @@ -367,16 +369,63 @@ def test_missing_check_results_hold_a_new_pr(self) -> None: self.assertEqual("author", outcome.route) self.assertTrue(outcome.facts.route_held_for_gates) - def test_maintenance_bot_hold_falls_back_to_approvers(self) -> None: - outcome = self.resolve( - { - "approval_count": 1, - "ci_pending_count": 1, - "is_maintenance_bot": True, + def test_opentelemetrybot_pr_never_routes_to_its_author(self) -> None: + facts = { + "author": "opentelemetrybot", + "ci_failing_count": 1, + "ci_pending_count": 0, + "is_maintenance_bot": True, + "author_can_act": False, + } + pending_actions = { + "thread": { + "action": "author", + "since": "2026-08-11T13:44:18Z", } + } + + awaiting_approval = self.resolve( + {**facts, "approval_count": 0}, + pending_actions, + required_approvals=2, + ) + approved = self.resolve( + {**facts, "approval_count": 1}, + pending_actions, + required_approvals=2, ) - self.assertEqual("approver", outcome.route) + self.assertEqual("approver", awaiting_approval.route) + self.assertEqual("maintainer", approved.route) + + def test_other_automation_uses_the_configured_approval_threshold(self) -> None: + facts = { + "author": "app/custom-automation", + "author_can_act": False, + "ci_failing_count": 1, + "ci_pending_count": 0, + "is_maintenance_bot": False, + } + pending_actions = { + "thread": { + "action": "author", + "since": "2026-08-11T13:44:18Z", + } + } + + one_approval = self.resolve( + {**facts, "approval_count": 1}, + pending_actions, + required_approvals=2, + ) + two_approvals = self.resolve( + {**facts, "approval_count": 2}, + pending_actions, + required_approvals=2, + ) + + self.assertEqual("approver", one_approval.route) + self.assertEqual("maintainer", two_approvals.route) def test_a_newly_classified_maintenance_bot_falls_back_to_approvers(self) -> None: # A cached result can still say "author" when the pull request author @@ -388,6 +437,7 @@ def test_a_newly_classified_maintenance_bot_falls_back_to_approvers(self) -> Non "ci_pending_count": 1, "head_sha": "abc", "is_maintenance_bot": True, + "author_can_act": False, }, previous_route="author", ) diff --git a/.github/scripts/pull-request-dashboard/test_state.py b/.github/scripts/pull-request-dashboard/test_state.py index dcb614ea6b1..dc16cade63d 100644 --- a/.github/scripts/pull-request-dashboard/test_state.py +++ b/.github/scripts/pull-request-dashboard/test_state.py @@ -221,6 +221,7 @@ def test_dashboard_facts_codec_round_trip(self) -> None: copilot_review_stale=True, copilot_review_needed=True, is_maintenance_bot=False, + author_can_act=True, is_draft=False, approval_count=2, conflicts="no", @@ -261,6 +262,128 @@ def test_dashboard_facts_codec_round_trip(self) -> None: decode_dashboard_facts(encode_dashboard_facts(facts)), ) + def test_legacy_facts_infer_whether_the_author_can_act(self) -> None: + cases = ( + ("alice", True), + ("app/dependabot", False), + ("renovate[bot]", False), + ("opentelemetrybot", False), + ) + for author, expected in cases: + with self.subTest(author=author): + self.assertEqual( + expected, + decode_dashboard_facts({"author": author}).author_can_act, + ) + + self.assertTrue( + decode_dashboard_facts({ + "author": "app/dependabot", + "author_can_act": True, + }).author_can_act + ) + self.assertEqual( + DashboardRoute.AUTHOR, + decode_stored_result( + { + "route": "author", + "facts": { + "author": "app/dependabot", + "author_can_act": True, + }, + }, + pr_number_hint=123, + ).route, + ) + + def test_version_thirteen_state_infers_author_capability(self) -> None: + persisted = { + "version": 13, + "initial_backfill_complete": True, + "prs": { + "123": { + "pr_number": 123, + "failed": False, + "route": "author", + "facts": {"author": "app/dependabot"}, + }, + "124": { + "pr_number": 124, + "failed": False, + "route": "approver", + "facts": {"author": "app/dependabot"}, + }, + "125": { + "pr_number": 125, + "failed": False, + "route": "author", + "facts": {"author": "alice"}, + }, + "126": { + "pr_number": 126, + "failed": False, + "route": "author", + "facts": { + "author": "app/dependabot", + "author_can_act": True, + }, + }, + }, + } + with ( + tempfile.TemporaryDirectory() as temp_dir, + patch("state._state_dir", Path(temp_dir)), + ): + dashboard_state_path().write_text( + json.dumps(persisted), + encoding="utf-8", + ) + + warnings = StringIO() + with redirect_stderr(warnings): + decoded = load_dashboard_state_cache() + + self.assertIsNotNone(decoded) + assert decoded is not None + self.assertEqual(frozenset({124, 125, 126}), decoded.pr_numbers) + decoded_by_number = { + result.pr_number: result + for result in decoded.results + } + self.assertFalse(decoded_by_number[124].facts.author_can_act) + self.assertEqual( + DashboardRoute.APPROVER, + decoded_by_number[124].route, + ) + self.assertTrue(decoded_by_number[125].facts.author_can_act) + self.assertEqual(DashboardRoute.AUTHOR, decoded_by_number[125].route) + self.assertTrue(decoded_by_number[126].facts.author_can_act) + self.assertEqual(DashboardRoute.AUTHOR, decoded_by_number[126].route) + self.assertIn( + "dashboard result author route requires an actionable author", + warnings.getvalue(), + ) + self.assertEqual( + DASHBOARD_STATE_VERSION, + encode_dashboard_state(decoded)["version"], + ) + + def test_version_fourteen_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": 14, + "initial_backfill_complete": True, + "prs": {}, + }), + encoding="utf-8", + ) + + self.assertIsNone(load_dashboard_state_cache()) + def test_stored_result_and_dashboard_state_codecs_round_trip(self) -> None: first = stored_dashboard_result( 7, @@ -514,19 +637,20 @@ def test_version_eleven_dashboard_state_migrates_to_current_shape(self) -> None: }, } - self.assertEqual( - { - **persisted, - "version": DASHBOARD_STATE_VERSION, - "draft_pr_numbers": [], - }, - encode_dashboard_state(decode_dashboard_state(persisted)), - ) + decoded = decode_dashboard_state(persisted) + self.assertTrue(decoded.results[0].facts.author_can_act) + expected = { + **persisted, + "version": DASHBOARD_STATE_VERSION, + "draft_pr_numbers": [], + } + expected["prs"]["123"]["facts"]["author_can_act"] = True + self.assertEqual(expected, encode_dashboard_state(decoded)) 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, 15) self.assertEqual(STATUS_COMMENT_ROLLOUT_STATE_VERSION, 2) self.assertEqual(AUTHOR_NUDGE_STATE_VERSION, 3) self.assertEqual(COPILOT_REVIEW_REQUEST_STATE_VERSION, 6) diff --git a/.github/scripts/pull-request-dashboard/utils.py b/.github/scripts/pull-request-dashboard/utils.py index ec394bfd72a..3dc7a730a79 100644 --- a/.github/scripts/pull-request-dashboard/utils.py +++ b/.github/scripts/pull-request-dashboard/utils.py @@ -66,6 +66,24 @@ def actor_login(obj: dict[str, Any] | None) -> str: return ((obj or {}).get("login") or "").strip() +def normalize_author_identity(login: str) -> str: + normalized = (login or "").strip().casefold() + if normalized.startswith("app/"): + return normalized.removeprefix("app/") + if normalized.endswith("[bot]"): + return normalized.removesuffix("[bot]") + return normalized + + +def is_unattended_author_login(login: str) -> bool: + normalized = (login or "").strip().casefold() + return ( + normalized == "opentelemetrybot" + or normalized.startswith("app/") + or normalized.endswith("[bot]") + ) + + # Every login GitHub has used for the Copilot reviewer, lowercased. COPILOT_REVIEWER_LOGINS = frozenset({ "copilot", diff --git a/pull-request-dashboard/README.md b/pull-request-dashboard/README.md index 7145f30a0f5..50017ed3778 100644 --- a/pull-request-dashboard/README.md +++ b/pull-request-dashboard/README.md @@ -205,8 +205,10 @@ results, the check GitHub publishes for each configured tool is treated as required, including its neutral "could not determine the alerts introduced by this pull request" outcome, which holds the merge even though GitHub does not mark that check as required. -Maintenance-bot PRs keep their -maintainer-oriented routing because the bot cannot act on a dashboard request. +PRs from unattended automation do not route to their author because the +automation cannot act on a dashboard request. A Copilot-delegated PR can route +to a human author recovered from an assignee or the first commit's committer. +Known maintenance bots still route to maintainers after one approval. A hidden marker lets the workflow update the comment in place and upgrade existing one-time guidance comments rather than creating duplicates. Status