From d602e818e1a0c74de4476fb5e03fd1b8959d013d Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Thu, 30 Jul 2026 12:53:14 -0700 Subject: [PATCH 01/11] Clear open items on a reviewer-routing override instead of masking the route --- .../pull-request-dashboard/dashboard.py | 9 +- .../dashboard_override.py | 129 ++++++++++++++--- .../pull-request-dashboard/test_dashboard.py | 10 ++ .../test_dashboard_override.py | 131 ++++++++++++++++++ pull-request-dashboard/README.md | 17 ++- 5 files changed, 267 insertions(+), 29 deletions(-) diff --git a/.github/scripts/pull-request-dashboard/dashboard.py b/.github/scripts/pull-request-dashboard/dashboard.py index ec0f4c2d6ea..9e9de88a0fa 100644 --- a/.github/scripts/pull-request-dashboard/dashboard.py +++ b/.github/scripts/pull-request-dashboard/dashboard.py @@ -198,6 +198,7 @@ from dashboard_override import ( apply_dashboard_override, append_route_noop_reply, + clear_overridden_actions, dashboard_command_body_remainder, dashboard_override_facts, ) @@ -1114,12 +1115,15 @@ def route_pr(facts: dict[str, Any], pending_actions: dict[str, dict[str, Any]], is_maintenance_bot = facts.get("is_maintenance_bot") approval_threshold = 1 if is_maintenance_bot else required_approvals # Precedence: - # 1. A required status check failure -> "author". + # 1. A required status check failure the author has not overridden -> "author". # 2. A discussion waiting on the author -> "author". # 3. If there are enough approvals and no inline or top-level feedback is # still waiting on a reviewer -> "maintainer". # 4. Otherwise the PR is still waiting on approvers. - if facts.get("ci_failing_count", 0) > 0 and not is_maintenance_bot: + ci_failing = facts.get("ci_failing_count", 0) > 0 and not facts.get( + "dashboard_override_cleared_ci" + ) + if ci_failing and not is_maintenance_bot: return "author" if counts["author"] and not is_maintenance_bot: return "author" @@ -1391,6 +1395,7 @@ def build_pr_result( author_comment_source_state, ) pending_actions = review_thread_pending_actions | top_level_pending_actions + pending_actions = clear_overridden_actions(facts, pending_actions) failed_classifications = [ classification for classification in ( diff --git a/.github/scripts/pull-request-dashboard/dashboard_override.py b/.github/scripts/pull-request-dashboard/dashboard_override.py index d9b383dd8de..a16faed01e0 100644 --- a/.github/scripts/pull-request-dashboard/dashboard_override.py +++ b/.github/scripts/pull-request-dashboard/dashboard_override.py @@ -29,7 +29,6 @@ r"" ) PRE_REVIEW_ROUTES = ("author",) -REVIEWERS_OR_LATER_ROUTES = ("approver", "maintainer") def author_override_guidance(staleness_note: str = "") -> str: @@ -105,6 +104,31 @@ def latest_authorized_command( return best_id, best_user +def latest_authorized_command_at( + raw: dict[str, Any], + author: str, + reviewers: set[str] | None, +) -> str: + """Timestamp of the newest authorized override command, acknowledged or not. + + Unlike `latest_authorized_command`, acknowledged commands still count: the + watermark has to outlive the acknowledgement, or the discussions a command + cleared would come back on the next refresh. + """ + latest = "" + for comment in raw.get("issue_comments") or []: + if parse_dashboard_command(comment) != DASHBOARD_OVERRIDE_SUBCOMMAND: + continue + if not is_authorized_commander( + actor_login(comment.get("user") or {}), author, reviewers + ): + continue + created_at = comment.get("created_at") or "" + if created_at > latest: + latest = created_at + return latest + + def dashboard_override_facts( raw: dict[str, Any], author: str, @@ -119,6 +143,9 @@ def dashboard_override_facts( "dashboard_override_label_applied": label_applied, "dashboard_override_command_id": command_id, "dashboard_override_command_user": command_user, + "dashboard_override_since": latest_authorized_command_at( + raw, author, reviewers + ), "dashboard_override_requested": command_pending and not label_applied, "dashboard_override_release_requested": False, "dashboard_command_replies": pending_command_replies(raw, author, reviewers), @@ -240,14 +267,21 @@ def render_command_reply(reply: dict[str, Any]) -> str: else: message = "routed this pull request to reviewers." elif kind == "already_routed": - where = ROUTE_ALREADY_ROUTED_PHRASE.get( - reply.get("route") or "", "not currently waiting on you" - ) - message = ( - f"this pull request is {where}, so `/dashboard route:reviewers` had " - "no effect. The command only applies while the pull request is " - "waiting on you." - ) + if reply.get("route") in PRE_REVIEW_ROUTES: + message = ( + "everything still open on this pull request arrived after your " + "`/dashboard route:reviewers` command, so it is still waiting " + "on you." + ) + else: + where = ROUTE_ALREADY_ROUTED_PHRASE.get( + reply.get("route") or "", "not currently waiting on you" + ) + message = ( + f"this pull request is {where}, so `/dashboard route:reviewers` had " + "no effect. The command only applies while the pull request is " + "waiting on you." + ) else: subcommand = reply.get("subcommand") or "" attempted = DASHBOARD_COMMAND_PREFIX + (f" {subcommand}" if subcommand else "") @@ -333,29 +367,82 @@ def deliver_dashboard_command_replies(repo: str) -> list[str]: return errors +def clear_overridden_actions( + facts: dict[str, Any], + pending_actions: dict[str, dict[str, Any]], +) -> dict[str, dict[str, Any]]: + """Drop the author actions an override command already answered. + + `/dashboard route:reviewers` is the author saying everything open at that + moment is handled. Clearing those actions, instead of only masking the route + once, keeps them from routing the pull request back to the author on every + later refresh, which would make the author repeat the command each time + review comes back around. Anything that happens after the command keeps its + author action, so new feedback still reaches the author. + """ + override_since = facts.get("dashboard_override_since") or "" + active = bool(facts.get("dashboard_override_label_applied")) or bool( + facts.get("dashboard_override_requested") + ) + facts["dashboard_override_cleared_count"] = 0 + facts["dashboard_override_cleared_ci"] = False + if not override_since or not active: + return pending_actions + remaining: dict[str, dict[str, Any]] = {} + cleared = 0 + for discussion_id, entry in pending_actions.items(): + since = entry.get("since") or "" + if entry.get("action") == "author" and since and since <= override_since: + cleared += 1 + continue + remaining[discussion_id] = entry + ci_failing_since = facts.get("ci_failing_since") or "" + facts["dashboard_override_cleared_count"] = cleared + facts["dashboard_override_cleared_ci"] = bool( + facts.get("ci_failing_count") + and ci_failing_since + and ci_failing_since <= override_since + ) + return remaining + + def apply_dashboard_override(facts: dict[str, Any], route: str) -> str: label_applied = bool(facts.get("dashboard_override_label_applied")) requested = bool(facts.get("dashboard_override_requested")) command_pending = bool(facts.get("dashboard_override_command_id")) - # The override only takes effect before review, while automatic routing waits - # on the author. On every later route the natural routing stands. - override_applies = route in PRE_REVIEW_ROUTES and (label_applied or requested) - # A command that does not newly move the pull request to reviewers is a no-op; - # the author is told where it is routed. This covers both a non-overridable - # route and an existing label that already provides the reviewer handoff. - facts["dashboard_override_noop"] = command_pending and ( + cleared = bool(facts.get("dashboard_override_cleared_count")) or bool( + facts.get("dashboard_override_cleared_ci") + ) + # `route` already reflects what `clear_overridden_actions` cleared, so a + # command needs no further route masking. Masking is left for a label applied + # by hand, which carries no command and so clears nothing; it only takes + # effect before review, and on every later route natural routing stands. + masks_route = ( + route in PRE_REVIEW_ROUTES + and (label_applied or requested) + and not facts.get("dashboard_override_since") + ) + override_applies = cleared or masks_route + # A command that does not newly move the pull request to reviewers is a + # no-op; the author is told where it is routed. This covers both a + # non-overridable route and an existing override that already provides the + # reviewer handoff. + facts["dashboard_override_noop"] = command_pending and not cleared and ( label_applied or not override_applies ) if requested and not override_applies: facts["dashboard_override_requested"] = False + elif command_pending and cleared and not requested: + # A command that cleared something while the label was already applied + # still needs its label refresh and acknowledgement reply. + facts["dashboard_override_requested"] = True facts["dashboard_override"] = override_applies - # Release the label once automatic routing reaches or passes reviewers, so a - # forgotten override cannot pin the pull request at reviewers or drag it back - # from maintainers. + # Release the label once the override stops clearing anything, so a + # forgotten override cannot keep new feedback away from the author. facts["dashboard_override_release_requested"] = ( - label_applied and route in REVIEWERS_OR_LATER_ROUTES + label_applied and not cleared and not masks_route ) - return "approver" if override_applies else route + return "approver" if masks_route else route def append_route_noop_reply( diff --git a/.github/scripts/pull-request-dashboard/test_dashboard.py b/.github/scripts/pull-request-dashboard/test_dashboard.py index 7a22edec160..5f28f96d5e0 100644 --- a/.github/scripts/pull-request-dashboard/test_dashboard.py +++ b/.github/scripts/pull-request-dashboard/test_dashboard.py @@ -896,6 +896,16 @@ def test_required_ci_failure_routes_to_author_before_approval_state(self) -> Non self.assertEqual("author", route_pr(facts, {}, 1)) + def test_override_cleared_ci_failure_does_not_route_to_author(self) -> None: + facts = { + "approval_count": 0, + "ci_failing_count": 1, + "is_maintenance_bot": False, + "dashboard_override_cleared_ci": True, + } + + self.assertEqual("approver", route_pr(facts, {}, 1)) + def test_required_ci_failure_preserves_maintenance_bot_routing(self) -> None: for approval_count, expected_route in ((0, "approver"), (1, "maintainer")): with self.subTest(approval_count=approval_count): diff --git a/.github/scripts/pull-request-dashboard/test_dashboard_override.py b/.github/scripts/pull-request-dashboard/test_dashboard_override.py index 5e07368b6df..969406894fd 100644 --- a/.github/scripts/pull-request-dashboard/test_dashboard_override.py +++ b/.github/scripts/pull-request-dashboard/test_dashboard_override.py @@ -194,6 +194,20 @@ def test_renders_already_routed_replies_per_route(self) -> None: self.assertIn(f"@author this pull request is {phrase}, so", body) self.assertIn("`/dashboard route:reviewers` had no effect", body) + def test_renders_already_routed_reply_for_a_command_that_cleared_nothing( + self, + ) -> None: + body = dashboard_override.render_command_reply( + {"comment_id": 7, "kind": "already_routed", "user": "author", "route": "author"} + ) + + self.assertIn(dashboard_override.override_ack_marker(7), body) + self.assertIn( + "@author everything still open on this pull request arrived after " + "your `/dashboard route:reviewers` command", + body, + ) + def test_appends_already_routed_reply_for_author_noop(self) -> None: facts = { "author": "author", @@ -442,6 +456,123 @@ def test_newest_acknowledgement_consumes_older_authorized_commands(self) -> None dashboard_override.latest_authorized_command(raw, "author", set()), ) + def test_watermark_survives_acknowledgement(self) -> None: + raw = { + "issue_comments": [ + { + "id": 3, + "user": {"login": "author"}, + "created_at": "2026-07-30T12:00:00Z", + "body": "/dashboard route:reviewers", + }, + { + "id": 4, + "user": {"login": "opentelemetry-pr-dashboard[bot]"}, + "created_at": "2026-07-30T12:05:00Z", + "body": dashboard_override.override_ack_marker(3), + }, + ] + } + + facts = dashboard_override.dashboard_override_facts(raw, "author", set()) + + self.assertEqual(0, facts["dashboard_override_command_id"]) + self.assertEqual("2026-07-30T12:00:00Z", facts["dashboard_override_since"]) + + def test_clears_only_author_actions_that_predate_the_command(self) -> None: + facts = { + "dashboard_override_label_applied": True, + "dashboard_override_since": "2026-07-30T12:00:00Z", + } + pending_actions = { + "old": {"action": "author", "since": "2026-07-30T11:00:00Z"}, + "new": {"action": "author", "since": "2026-07-30T13:00:00Z"}, + "reviewer": {"action": "reviewer", "since": "2026-07-30T11:00:00Z"}, + } + + remaining = dashboard_override.clear_overridden_actions(facts, pending_actions) + + self.assertEqual(["new", "reviewer"], sorted(remaining)) + self.assertEqual(1, facts["dashboard_override_cleared_count"]) + + def test_clears_check_failures_that_predate_the_command(self) -> None: + for ci_failing_since, expected in ( + ("2026-07-30T11:00:00Z", True), + ("2026-07-30T13:00:00Z", False), + ): + with self.subTest(ci_failing_since=ci_failing_since): + facts = { + "dashboard_override_label_applied": True, + "dashboard_override_since": "2026-07-30T12:00:00Z", + "ci_failing_count": 1, + "ci_failing_since": ci_failing_since, + } + + dashboard_override.clear_overridden_actions(facts, {}) + + self.assertEqual(expected, facts["dashboard_override_cleared_ci"]) + + def test_label_removal_restores_cleared_actions(self) -> None: + facts = { + "dashboard_override_label_applied": False, + "dashboard_override_requested": False, + "dashboard_override_since": "2026-07-30T12:00:00Z", + } + pending_actions = {"old": {"action": "author", "since": "2026-07-30T11:00:00Z"}} + + remaining = dashboard_override.clear_overridden_actions(facts, pending_actions) + + self.assertEqual(pending_actions, remaining) + self.assertEqual(0, facts["dashboard_override_cleared_count"]) + + def test_cleared_command_is_acknowledged_without_masking_the_route(self) -> None: + facts = { + "dashboard_override_label_applied": False, + "dashboard_override_requested": True, + "dashboard_override_command_id": 5, + "dashboard_override_since": "2026-07-30T12:00:00Z", + "dashboard_override_cleared_count": 1, + } + + route = dashboard_override.apply_dashboard_override(facts, "maintainer") + + self.assertEqual("maintainer", route) + self.assertTrue(facts["dashboard_override"]) + self.assertTrue(facts["dashboard_override_requested"]) + self.assertFalse(facts["dashboard_override_noop"]) + self.assertFalse(facts["dashboard_override_release_requested"]) + + def test_command_arriving_with_the_label_present_is_acknowledged_when_it_clears( + self, + ) -> None: + facts = { + "dashboard_override_label_applied": True, + "dashboard_override_requested": False, + "dashboard_override_command_id": 5, + "dashboard_override_since": "2026-07-30T12:00:00Z", + "dashboard_override_cleared_count": 1, + } + + route = dashboard_override.apply_dashboard_override(facts, "approver") + + self.assertEqual("approver", route) + self.assertTrue(facts["dashboard_override_requested"]) + self.assertFalse(facts["dashboard_override_noop"]) + + def test_new_feedback_after_the_command_reaches_the_author(self) -> None: + facts = { + "dashboard_override_label_applied": True, + "dashboard_override_requested": False, + "dashboard_override_since": "2026-07-30T12:00:00Z", + "dashboard_override_cleared_count": 0, + } + + route = dashboard_override.apply_dashboard_override(facts, "author") + + self.assertEqual("author", route) + self.assertFalse(facts["dashboard_override"]) + self.assertTrue(facts["dashboard_override_release_requested"]) + def test_fresh_command_with_label_present_is_a_noop(self) -> None: facts = { "dashboard_override_label_applied": True, diff --git a/pull-request-dashboard/README.md b/pull-request-dashboard/README.md index 44d7e4e2c4b..8ab03594b61 100644 --- a/pull-request-dashboard/README.md +++ b/pull-request-dashboard/README.md @@ -183,8 +183,11 @@ Targeted updates received before the first full dashboard run are ignored. When the dashboard says a pull request is waiting on its author but the author believes it is ready for another review, the author -can comment `/dashboard route:reviewers`. The dashboard routes the pull request -to *Waiting on reviewers* and applies the `dashboard:route-overridden` label to +can comment `/dashboard route:reviewers`. The command clears every review item +and required check failure that was already open when it was posted, so the +pull request routes to *Waiting on reviewers* and stays there instead of falling +back to the author on the next refresh. The dashboard applies the +`dashboard:route-overridden` label to mark the override, which the dashboard shows inline after the PR title. Members of the repository's `approver_teams` can use the same command. A `/dashboard route:reviewers` command from anyone else, or a command on a pull request already at or past reviewers, has no routing effect. The @@ -193,10 +196,12 @@ explaining that only the author or an approver can use it, replies to an author or approver command on a pull request already at or past reviewers noting where it is currently routed, and replies to any unrecognized `/dashboard` command. -Removing the `dashboard:route-overridden` label restores automatic routing. The -dashboard also removes the label automatically once routing would place the pull -request with reviewers on its own, so a forgotten override does not linger. A -command that has already been applied is not replayed after label removal; the +Anything that happens after the command keeps its author action, so new review +feedback and new check failures still route the pull request back to the author. +Removing the `dashboard:route-overridden` label restores automatic routing for +the cleared items too. The dashboard also removes the label automatically once +the override no longer clears anything, so a forgotten override does not linger. +A command that has already been applied is not replayed after label removal; the author can post a new `/dashboard route:reviewers` command if another override is needed later. From 701ce216c98bffb21632b448d5fcbb27a57c6d5d Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Thu, 30 Jul 2026 13:16:50 -0700 Subject: [PATCH 02/11] Drop the route-override label and derive clearing from the command alone --- .../pull-request-dashboard/author_nudge.py | 7 - .../pull-request-dashboard/dashboard.py | 20 +- .../dashboard_override.py | 190 ++-------- .../pull-request-dashboard/delivery.py | 10 +- .../scripts/pull-request-dashboard/render.py | 7 +- .../test_author_nudge.py | 26 +- .../test_copilot_review.py | 2 - .../pull-request-dashboard/test_dashboard.py | 6 +- .../test_dashboard_override.py | 348 +++--------------- .../pull-request-dashboard/test_delivery.py | 11 +- .../pull-request-dashboard/test_render.py | 20 - pull-request-dashboard/README.md | 23 +- 12 files changed, 97 insertions(+), 573 deletions(-) diff --git a/.github/scripts/pull-request-dashboard/author_nudge.py b/.github/scripts/pull-request-dashboard/author_nudge.py index 4be719cb44a..990010f8587 100644 --- a/.github/scripts/pull-request-dashboard/author_nudge.py +++ b/.github/scripts/pull-request-dashboard/author_nudge.py @@ -16,7 +16,6 @@ run_gh, ) from dashboard_override import author_override_guidance -from dashboard_override import DASHBOARD_OVERRIDE_LABEL from pr_status_comment import ( DASHBOARD_APP_SLUG, managed_status_comments, @@ -50,12 +49,6 @@ def routing_inputs(raw: dict[str, Any]) -> dict[str, Any]: "base_branch": str(pr.get("baseRefName") or ""), "checks": raw.get("checks"), "issue_comments": issue_comments, - "labels": sorted( - label.get("name") or "" - for label in pr.get("labels") or [] - if isinstance(label, dict) - and label.get("name") == DASHBOARD_OVERRIDE_LABEL - ), "pr_text": { "body": str(pr.get("body") or "").replace("\r\n", "\n"), "title": str(pr.get("title") or ""), diff --git a/.github/scripts/pull-request-dashboard/dashboard.py b/.github/scripts/pull-request-dashboard/dashboard.py index 9e9de88a0fa..4c1b6387a45 100644 --- a/.github/scripts/pull-request-dashboard/dashboard.py +++ b/.github/scripts/pull-request-dashboard/dashboard.py @@ -196,8 +196,7 @@ record_copilot_review_observation, ) from dashboard_override import ( - apply_dashboard_override, - append_route_noop_reply, + append_command_ack_reply, clear_overridden_actions, dashboard_command_body_remainder, dashboard_override_facts, @@ -543,17 +542,12 @@ def compute_facts( raw.get("reviews") or [], head_sha, ) - labels = { - label.get("name") or "" - for label in pr.get("labels") or [] - if isinstance(label, dict) - } facts = { "author": author, "assignees": assignees, "head_sha": head_sha, "routing_input_fingerprint": routing_input_fingerprint(raw), - **dashboard_override_facts(raw, author, labels, reviewers or set()), + **dashboard_override_facts(raw, author, reviewers or set()), "copilot_review_requested": any( is_copilot_reviewer(request) for request in (raw.get("review_requests") or []) @@ -1292,15 +1286,9 @@ def resolve_pr_route( required_approvals: int, require_clean_copilot_review: bool, ) -> str: - # Apply the manual reviewer-routing override before the Copilot review gate - # so an overridden route (for example author -> reviewers) is still held for - # a required clean Copilot review instead of bypassing it. return apply_copilot_review_gate( facts, - apply_dashboard_override( - facts, - route_pr(facts, pending_actions, required_approvals), - ), + route_pr(facts, pending_actions, required_approvals), enabled=require_clean_copilot_review, ) @@ -1438,7 +1426,7 @@ def build_pr_result( previous_result, raw.get("issue_comments") or [], ) - append_route_noop_reply(raw, facts, route) + append_command_ack_reply(raw, facts, route) add_wait_age_facts(facts, route, pending_actions) facts["author_action_review_thread_urls"] = author_action_discussion_urls( review_threads, pending_actions diff --git a/.github/scripts/pull-request-dashboard/dashboard_override.py b/.github/scripts/pull-request-dashboard/dashboard_override.py index a16faed01e0..822edac3f68 100644 --- a/.github/scripts/pull-request-dashboard/dashboard_override.py +++ b/.github/scripts/pull-request-dashboard/dashboard_override.py @@ -4,19 +4,15 @@ import re from typing import Any -from urllib.parse import quote from github_cli import gh_api, run_gh from state import load_dashboard_state_cache from utils import actor_login -DASHBOARD_OVERRIDE_LABEL = "dashboard:route-overridden" DASHBOARD_COMMAND_PREFIX = "/dashboard" DASHBOARD_OVERRIDE_COMMAND = "/dashboard route:reviewers" DASHBOARD_OVERRIDE_SUBCOMMAND = "route:reviewers" -DASHBOARD_OVERRIDE_LABEL_COLOR = "1D76DB" -DASHBOARD_OVERRIDE_LABEL_DESCRIPTION = "Routing manually overridden to reviewers" # Mirrors pr_status_comment.DASHBOARD_APP_SLUG; duplicated here to avoid a # circular import between the two modules. DASHBOARD_APP_SLUG = "opentelemetry-pr-dashboard" @@ -132,22 +128,15 @@ def latest_authorized_command_at( def dashboard_override_facts( raw: dict[str, Any], author: str, - labels: set[str], reviewers: set[str] | None = None, ) -> dict[str, Any]: - label_applied = DASHBOARD_OVERRIDE_LABEL in labels command_id, command_user = latest_authorized_command(raw, author, reviewers) - command_pending = bool(command_id) return { - "dashboard_override": label_applied, - "dashboard_override_label_applied": label_applied, "dashboard_override_command_id": command_id, "dashboard_override_command_user": command_user, "dashboard_override_since": latest_authorized_command_at( raw, author, reviewers ), - "dashboard_override_requested": command_pending and not label_applied, - "dashboard_override_release_requested": False, "dashboard_command_replies": pending_command_replies(raw, author, reviewers), } @@ -258,30 +247,29 @@ def render_command_reply(reply: dict[str, Any]) -> str: "only the pull request author or a member of an approving team can " "use `/dashboard route:reviewers`." ) - elif kind == "routed": - if reply.get("route") == "copilot": - message = ( - "accepted the reviewer-routing override; the reviewer handoff " - "is waiting on Copilot." - ) - else: - message = "routed this pull request to reviewers." - elif kind == "already_routed": - if reply.get("route") in PRE_REVIEW_ROUTES: + elif kind in ("routed", "already_routed"): + route = reply.get("route") or "" + if route in PRE_REVIEW_ROUTES: message = ( "everything still open on this pull request arrived after your " "`/dashboard route:reviewers` command, so it is still waiting " "on you." ) - else: + elif kind == "already_routed": where = ROUTE_ALREADY_ROUTED_PHRASE.get( - reply.get("route") or "", "not currently waiting on you" + route, "not currently waiting on you" ) message = ( f"this pull request is {where}, so `/dashboard route:reviewers` had " - "no effect. The command only applies while the pull request is " - "waiting on you." + "no effect." ) + elif route == "copilot": + message = ( + "accepted the reviewer-routing override; the reviewer handoff " + "is waiting on Copilot." + ) + else: + message = "routed this pull request to reviewers." else: subcommand = reply.get("subcommand") or "" attempted = DASHBOARD_COMMAND_PREFIX + (f" {subcommand}" if subcommand else "") @@ -314,24 +302,6 @@ def command_reply_exists( ) -def ensure_command_reply( - repo: str, - pr_number: int, - reply: dict[str, Any], -) -> None: - comments = gh_api( - f"/repos/{repo}/issues/{pr_number}/comments?per_page=100", - paginate=True, - ) - if command_reply_exists(comments, int(reply["comment_id"])): - return - run_gh([ - "gh", "api", "--method", "POST", - f"repos/{repo}/issues/{pr_number}/comments", - "-f", f"body={render_command_reply(reply)}", - ]) - - def deliver_dashboard_command_replies(repo: str) -> list[str]: dashboard_state = load_dashboard_state_cache() if dashboard_state is None: @@ -374,19 +344,17 @@ def clear_overridden_actions( """Drop the author actions an override command already answered. `/dashboard route:reviewers` is the author saying everything open at that - moment is handled. Clearing those actions, instead of only masking the route - once, keeps them from routing the pull request back to the author on every - later refresh, which would make the author repeat the command each time - review comes back around. Anything that happens after the command keeps its - author action, so new feedback still reaches the author. + moment is handled. Clearing those actions keeps them from routing the pull + request back to the author on every later refresh, which would make the + author repeat the command each time review comes back around. Anything that + happens after the command keeps its author action, so new feedback still + reaches the author, including a reviewer reopening something the command + cleared. """ override_since = facts.get("dashboard_override_since") or "" - active = bool(facts.get("dashboard_override_label_applied")) or bool( - facts.get("dashboard_override_requested") - ) facts["dashboard_override_cleared_count"] = 0 facts["dashboard_override_cleared_ci"] = False - if not override_since or not active: + if not override_since: return pending_actions remaining: dict[str, dict[str, Any]] = {} cleared = 0 @@ -406,122 +374,32 @@ def clear_overridden_actions( return remaining -def apply_dashboard_override(facts: dict[str, Any], route: str) -> str: - label_applied = bool(facts.get("dashboard_override_label_applied")) - requested = bool(facts.get("dashboard_override_requested")) - command_pending = bool(facts.get("dashboard_override_command_id")) - cleared = bool(facts.get("dashboard_override_cleared_count")) or bool( - facts.get("dashboard_override_cleared_ci") - ) - # `route` already reflects what `clear_overridden_actions` cleared, so a - # command needs no further route masking. Masking is left for a label applied - # by hand, which carries no command and so clears nothing; it only takes - # effect before review, and on every later route natural routing stands. - masks_route = ( - route in PRE_REVIEW_ROUTES - and (label_applied or requested) - and not facts.get("dashboard_override_since") - ) - override_applies = cleared or masks_route - # A command that does not newly move the pull request to reviewers is a - # no-op; the author is told where it is routed. This covers both a - # non-overridable route and an existing override that already provides the - # reviewer handoff. - facts["dashboard_override_noop"] = command_pending and not cleared and ( - label_applied or not override_applies - ) - if requested and not override_applies: - facts["dashboard_override_requested"] = False - elif command_pending and cleared and not requested: - # A command that cleared something while the label was already applied - # still needs its label refresh and acknowledgement reply. - facts["dashboard_override_requested"] = True - facts["dashboard_override"] = override_applies - # Release the label once the override stops clearing anything, so a - # forgotten override cannot keep new feedback away from the author. - facts["dashboard_override_release_requested"] = ( - label_applied and not cleared and not masks_route - ) - return "approver" if masks_route else route - - -def append_route_noop_reply( +def append_command_ack_reply( raw: dict[str, Any], facts: dict[str, Any], route: str, ) -> None: - if not facts.get("dashboard_override_noop"): - return + """Queue the reply that acknowledges an override command. + + The reply carries the acknowledgement marker that stops the command from + being processed again, so every command gets one, whether or not it cleared + anything. `route` is already what the cleared items produced, so a command + that cleared nothing is answered with where the pull request is routed. + """ command_id = int(facts.get("dashboard_override_command_id") or 0) if not command_id: return - replied_ids = _replied_command_ids(raw.get("issue_comments") or []) - if command_id in replied_ids: + if command_id in _replied_command_ids(raw.get("issue_comments") or []): return replies = facts.setdefault("dashboard_command_replies", []) if any(reply.get("comment_id") == command_id for reply in replies): return + cleared = bool(facts.get("dashboard_override_cleared_count")) or bool( + facts.get("dashboard_override_cleared_ci") + ) replies.append({ "comment_id": command_id, - "kind": "already_routed", + "kind": "routed" if cleared else "already_routed", "user": facts.get("dashboard_override_command_user") or facts.get("author") or "", "route": route, - }) - - -def deliver_dashboard_override_requests(repo: str) -> list[str]: - dashboard_state = load_dashboard_state_cache() - if dashboard_state is None: - return [] - pr_results = sorted( - (dashboard_state.get("prs") or {}).items(), - key=lambda item: int(item[0]), - ) - if any( - ((result or {}).get("facts") or {}).get("dashboard_override_requested") - for _key, result in pr_results - ): - try: - run_gh([ - "gh", "label", "create", DASHBOARD_OVERRIDE_LABEL, - "--repo", repo, - "--color", DASHBOARD_OVERRIDE_LABEL_COLOR, - "--description", DASHBOARD_OVERRIDE_LABEL_DESCRIPTION, - "--force", - ]) - except Exception as e: - return [f"label: {e}"] - - errors: list[str] = [] - for key, result in pr_results: - facts = (result or {}).get("facts") or {} - pr_number = int(key) - if facts.get("dashboard_override_requested"): - try: - run_gh([ - "gh", "api", "--method", "POST", - f"repos/{repo}/issues/{pr_number}/labels", - "-f", f"labels[]={DASHBOARD_OVERRIDE_LABEL}", - ]) - ensure_command_reply( - repo, - pr_number, - { - "comment_id": facts["dashboard_override_command_id"], - "kind": "routed", - "route": (result or {}).get("route") or "", - "user": facts.get("dashboard_override_command_user") or "", - }, - ) - except Exception as e: - errors.append(f"PR #{pr_number}: {e}") - elif facts.get("dashboard_override_release_requested"): - try: - run_gh([ - "gh", "api", "--method", "DELETE", - f"repos/{repo}/issues/{pr_number}/labels/{quote(DASHBOARD_OVERRIDE_LABEL)}", - ]) - except Exception as e: - if "404" not in str(e): - errors.append(f"PR #{pr_number}: {e}") - return errors \ No newline at end of file + }) \ No newline at end of file diff --git a/.github/scripts/pull-request-dashboard/delivery.py b/.github/scripts/pull-request-dashboard/delivery.py index 642d963f4cb..ed5d5c6620f 100644 --- a/.github/scripts/pull-request-dashboard/delivery.py +++ b/.github/scripts/pull-request-dashboard/delivery.py @@ -12,10 +12,7 @@ from author_nudge import deliver_prepared_author_nudges from copilot_review import deliver_copilot_review_requests -from dashboard_override import ( - deliver_dashboard_command_replies, - deliver_dashboard_override_requests, -) +from dashboard_override import deliver_dashboard_command_replies from github_cli import detect_repo, gh_api, list_open_prs, normalize_repo, repo_state_key from notify_slack import notify_slack_from_state from pr_status_comment import ( @@ -74,11 +71,6 @@ def deliver_from_state( except Exception as e: errors.append(f"open pull requests: {e}") open_prs = None - run_delivery_action( - "dashboard overrides", - lambda: deliver_dashboard_override_requests(repo), - errors, - ) run_delivery_action( "dashboard command replies", lambda: deliver_dashboard_command_replies(repo), diff --git a/.github/scripts/pull-request-dashboard/render.py b/.github/scripts/pull-request-dashboard/render.py index c0a5428f833..e5c1306f6bc 100644 --- a/.github/scripts/pull-request-dashboard/render.py +++ b/.github/scripts/pull-request-dashboard/render.py @@ -8,11 +8,6 @@ from utils import actor_login, activity_age, markdown_escape, parse_ts, seconds_since -# Mirrors dashboard_override.DASHBOARD_OVERRIDE_LABEL; duplicated here to keep -# rendering free of that module's GitHub CLI dependencies. -DASHBOARD_OVERRIDE_LABEL = "dashboard:route-overridden" - - def _limit_rows(rows: list[Any], max_rows: int | None) -> tuple[list[Any], int]: if max_rows is None or max_rows <= 0 or len(rows) <= max_rows: return rows, 0 @@ -31,7 +26,7 @@ def _pr_cell_text( number = pr["number"] title = markdown_escape(pr.get("title", "")) pr_cell = f"#{number} {title}" - patterns = [*(labels_to_display or []), DASHBOARD_OVERRIDE_LABEL] + patterns = labels_to_display or [] matched_labels: list[str] = [] seen: set[str] = set() diff --git a/.github/scripts/pull-request-dashboard/test_author_nudge.py b/.github/scripts/pull-request-dashboard/test_author_nudge.py index eabb2ecad48..29146f5209e 100644 --- a/.github/scripts/pull-request-dashboard/test_author_nudge.py +++ b/.github/scripts/pull-request-dashboard/test_author_nudge.py @@ -47,27 +47,6 @@ def test_routing_fingerprint_ignores_dashboard_comments_and_tracks_author_activi }) self.assertNotEqual(baseline, author_nudge.routing_input_fingerprint(raw)) - def test_routing_fingerprint_tracks_normalized_labels(self) -> None: - raw = { - "checks": [], - "issue_comments": [], - "pr": {"labels": [{"name": "needs-triage"}]}, - "review_comments": [], - "reviews": [], - "review_threads": [], - } - baseline = author_nudge.routing_input_fingerprint(raw) - raw["pr"]["labels"].append({"name": "documentation"}) - - self.assertEqual(baseline, author_nudge.routing_input_fingerprint(raw)) - - raw["pr"]["labels"].append({"name": "dashboard:route-overridden"}) - overridden = author_nudge.routing_input_fingerprint(raw) - - self.assertNotEqual(baseline, overridden) - raw["pr"]["labels"].reverse() - self.assertEqual(overridden, author_nudge.routing_input_fingerprint(raw)) - def test_routing_fingerprint_tracks_required_check_state(self) -> None: raw = { "checks": [{"name": "build", "bucket": "fail"}], @@ -140,10 +119,7 @@ def test_fetch_current_routing_state_includes_required_checks( "baseRefName": "main", "body": "Current body", "headRefOid": "current-head", - "labels": [ - {"name": "needs-triage"}, - {"name": "dashboard:route-overridden"}, - ], + "labels": [{"name": "needs-triage"}], "title": "Current title", } gh_pr_view.return_value = pr diff --git a/.github/scripts/pull-request-dashboard/test_copilot_review.py b/.github/scripts/pull-request-dashboard/test_copilot_review.py index 8b04aec909e..820303a0a81 100644 --- a/.github/scripts/pull-request-dashboard/test_copilot_review.py +++ b/.github/scripts/pull-request-dashboard/test_copilot_review.py @@ -334,7 +334,6 @@ def test_drops_request_when_live_routing_inputs_changed( "base_branch", "checks", "issue_comments", - "labels", "pr_text", "review_comments", "reviews", @@ -459,7 +458,6 @@ def test_fingerprint_mismatch_reports_component_digests(self) -> None: "base_branch", "checks", "issue_comments", - "labels", "pr_text", "review_comments", "reviews", diff --git a/.github/scripts/pull-request-dashboard/test_dashboard.py b/.github/scripts/pull-request-dashboard/test_dashboard.py index 5f28f96d5e0..033011af3e0 100644 --- a/.github/scripts/pull-request-dashboard/test_dashboard.py +++ b/.github/scripts/pull-request-dashboard/test_dashboard.py @@ -32,11 +32,11 @@ class ResolvePrRouteTest(unittest.TestCase): def _author_route_facts(self, **overrides: object) -> dict[str, object]: - # A failing required check routes a human-authored PR to the author. + # A failing required check routes a human-authored PR to the author + # unless an override already cleared it. facts: dict[str, object] = { "ci_failing_count": 1, - "dashboard_override_label_applied": True, - "dashboard_override_requested": False, + "dashboard_override_cleared_ci": True, } facts.update(overrides) return facts diff --git a/.github/scripts/pull-request-dashboard/test_dashboard_override.py b/.github/scripts/pull-request-dashboard/test_dashboard_override.py index 969406894fd..239648bd1f1 100644 --- a/.github/scripts/pull-request-dashboard/test_dashboard_override.py +++ b/.github/scripts/pull-request-dashboard/test_dashboard_override.py @@ -208,35 +208,43 @@ def test_renders_already_routed_reply_for_a_command_that_cleared_nothing( body, ) - def test_appends_already_routed_reply_for_author_noop(self) -> None: + def test_appends_already_routed_reply_for_a_command_that_cleared_nothing(self) -> None: facts = { "author": "author", - "dashboard_override_noop": True, "dashboard_override_command_id": 12, } - dashboard_override.append_route_noop_reply({"issue_comments": []}, facts, "approver") + dashboard_override.append_command_ack_reply({"issue_comments": []}, facts, "approver") self.assertEqual( [{"comment_id": 12, "kind": "already_routed", "user": "author", "route": "approver"}], facts["dashboard_command_replies"], ) - def test_no_already_routed_reply_when_command_applies(self) -> None: + def test_appends_routed_reply_when_the_command_cleared_something(self) -> None: facts = { "author": "author", - "dashboard_override_noop": False, "dashboard_override_command_id": 12, + "dashboard_override_cleared_count": 1, } - dashboard_override.append_route_noop_reply({"issue_comments": []}, facts, "author") + dashboard_override.append_command_ack_reply({"issue_comments": []}, facts, "approver") + + self.assertEqual( + [{"comment_id": 12, "kind": "routed", "user": "author", "route": "approver"}], + facts["dashboard_command_replies"], + ) + + def test_no_ack_reply_without_a_pending_command(self) -> None: + facts = {"author": "author", "dashboard_override_command_id": 0} + + dashboard_override.append_command_ack_reply({"issue_comments": []}, facts, "author") self.assertNotIn("dashboard_command_replies", facts) def test_already_routed_reply_deduped_by_existing_marker(self) -> None: facts = { "author": "author", - "dashboard_override_noop": True, "dashboard_override_command_id": 12, } raw = { @@ -248,14 +256,13 @@ def test_already_routed_reply_deduped_by_existing_marker(self) -> None: ] } - dashboard_override.append_route_noop_reply(raw, facts, "approver") + dashboard_override.append_command_ack_reply(raw, facts, "approver") self.assertNotIn("dashboard_command_replies", facts) def test_forged_marker_does_not_dedupe_already_routed_reply(self) -> None: facts = { "author": "author", - "dashboard_override_noop": True, "dashboard_override_command_id": 12, } raw = { @@ -267,7 +274,7 @@ def test_forged_marker_does_not_dedupe_already_routed_reply(self) -> None: ] } - dashboard_override.append_route_noop_reply(raw, facts, "approver") + dashboard_override.append_command_ack_reply(raw, facts, "approver") self.assertEqual( [{"comment_id": 12, "kind": "already_routed", "user": "author", "route": "approver"}], @@ -346,13 +353,8 @@ def test_command_stays_pending_until_app_acknowledges_it(self) -> None: ] } - first = dashboard_override.dashboard_override_facts(raw, "author", set()) - retry = dashboard_override.dashboard_override_facts(raw, "author", set()) - label_observed = dashboard_override.dashboard_override_facts( - raw, - "author", - {"dashboard:route-overridden"}, - ) + first = dashboard_override.dashboard_override_facts(raw, "author") + retry = dashboard_override.dashboard_override_facts(raw, "author") acknowledged_raw = { "issue_comments": [ *raw["issue_comments"], @@ -366,16 +368,11 @@ def test_command_stays_pending_until_app_acknowledges_it(self) -> None: acknowledged = dashboard_override.dashboard_override_facts( acknowledged_raw, "author", - set(), ) - self.assertTrue(first["dashboard_override_requested"]) - self.assertTrue(retry["dashboard_override_requested"]) - self.assertTrue(label_observed["dashboard_override_label_applied"]) - self.assertEqual(3, label_observed["dashboard_override_command_id"]) - self.assertFalse(label_observed["dashboard_override_requested"]) + self.assertEqual(3, first["dashboard_override_command_id"]) + self.assertEqual(3, retry["dashboard_override_command_id"]) self.assertEqual(0, acknowledged["dashboard_override_command_id"]) - self.assertFalse(acknowledged["dashboard_override_requested"]) def test_newer_command_reapplies_removed_override(self) -> None: raw = { @@ -390,13 +387,8 @@ def test_newer_command_reapplies_removed_override(self) -> None: ] } - facts = dashboard_override.dashboard_override_facts( - raw, - "author", - set(), - ) + facts = dashboard_override.dashboard_override_facts(raw, "author") - self.assertTrue(facts["dashboard_override_requested"]) self.assertEqual(5, facts["dashboard_override_command_id"]) def test_rebuilds_unacknowledged_already_routed_reply_across_refreshes(self) -> None: @@ -407,9 +399,8 @@ def test_rebuilds_unacknowledged_already_routed_reply_across_refreshes(self) -> } for _ in range(2): - facts = dashboard_override.dashboard_override_facts(raw, "author", set()) - route = dashboard_override.apply_dashboard_override(facts, "approver") - dashboard_override.append_route_noop_reply(raw, facts, route) + facts = dashboard_override.dashboard_override_facts(raw, "author") + dashboard_override.append_command_ack_reply(raw, facts, "approver") self.assertEqual( [{"comment_id": 5, "kind": "already_routed", "user": "author", "route": "approver"}], @@ -428,14 +419,9 @@ def test_acknowledged_command_does_not_replay_after_cache_eviction(self) -> None ] } - facts = dashboard_override.dashboard_override_facts( - raw, - "author", - set(), - ) + facts = dashboard_override.dashboard_override_facts(raw, "author") self.assertEqual(0, facts["dashboard_override_command_id"]) - self.assertFalse(facts["dashboard_override_requested"]) self.assertEqual([], facts["dashboard_command_replies"]) def test_newest_acknowledgement_consumes_older_authorized_commands(self) -> None: @@ -474,16 +460,13 @@ def test_watermark_survives_acknowledgement(self) -> None: ] } - facts = dashboard_override.dashboard_override_facts(raw, "author", set()) + facts = dashboard_override.dashboard_override_facts(raw, "author") self.assertEqual(0, facts["dashboard_override_command_id"]) self.assertEqual("2026-07-30T12:00:00Z", facts["dashboard_override_since"]) def test_clears_only_author_actions_that_predate_the_command(self) -> None: - facts = { - "dashboard_override_label_applied": True, - "dashboard_override_since": "2026-07-30T12:00:00Z", - } + facts = {"dashboard_override_since": "2026-07-30T12:00:00Z"} pending_actions = { "old": {"action": "author", "since": "2026-07-30T11:00:00Z"}, "new": {"action": "author", "since": "2026-07-30T13:00:00Z"}, @@ -502,7 +485,6 @@ def test_clears_check_failures_that_predate_the_command(self) -> None: ): with self.subTest(ci_failing_since=ci_failing_since): facts = { - "dashboard_override_label_applied": True, "dashboard_override_since": "2026-07-30T12:00:00Z", "ci_failing_count": 1, "ci_failing_since": ci_failing_since, @@ -512,12 +494,8 @@ def test_clears_check_failures_that_predate_the_command(self) -> None: self.assertEqual(expected, facts["dashboard_override_cleared_ci"]) - def test_label_removal_restores_cleared_actions(self) -> None: - facts = { - "dashboard_override_label_applied": False, - "dashboard_override_requested": False, - "dashboard_override_since": "2026-07-30T12:00:00Z", - } + def test_clears_nothing_without_a_command(self) -> None: + facts = {"dashboard_override_since": ""} pending_actions = {"old": {"action": "author", "since": "2026-07-30T11:00:00Z"}} remaining = dashboard_override.clear_overridden_actions(facts, pending_actions) @@ -525,194 +503,21 @@ def test_label_removal_restores_cleared_actions(self) -> None: self.assertEqual(pending_actions, remaining) self.assertEqual(0, facts["dashboard_override_cleared_count"]) - def test_cleared_command_is_acknowledged_without_masking_the_route(self) -> None: - facts = { - "dashboard_override_label_applied": False, - "dashboard_override_requested": True, - "dashboard_override_command_id": 5, - "dashboard_override_since": "2026-07-30T12:00:00Z", - "dashboard_override_cleared_count": 1, - } - - route = dashboard_override.apply_dashboard_override(facts, "maintainer") - - self.assertEqual("maintainer", route) - self.assertTrue(facts["dashboard_override"]) - self.assertTrue(facts["dashboard_override_requested"]) - self.assertFalse(facts["dashboard_override_noop"]) - self.assertFalse(facts["dashboard_override_release_requested"]) - - def test_command_arriving_with_the_label_present_is_acknowledged_when_it_clears( - self, - ) -> None: - facts = { - "dashboard_override_label_applied": True, - "dashboard_override_requested": False, - "dashboard_override_command_id": 5, - "dashboard_override_since": "2026-07-30T12:00:00Z", - "dashboard_override_cleared_count": 1, - } - - route = dashboard_override.apply_dashboard_override(facts, "approver") - - self.assertEqual("approver", route) - self.assertTrue(facts["dashboard_override_requested"]) - self.assertFalse(facts["dashboard_override_noop"]) - - def test_new_feedback_after_the_command_reaches_the_author(self) -> None: - facts = { - "dashboard_override_label_applied": True, - "dashboard_override_requested": False, - "dashboard_override_since": "2026-07-30T12:00:00Z", - "dashboard_override_cleared_count": 0, - } - - route = dashboard_override.apply_dashboard_override(facts, "author") - - self.assertEqual("author", route) - self.assertFalse(facts["dashboard_override"]) - self.assertTrue(facts["dashboard_override_release_requested"]) - - def test_fresh_command_with_label_present_is_a_noop(self) -> None: - facts = { - "dashboard_override_label_applied": True, - "dashboard_override_requested": False, - "dashboard_override_command_id": 5, - } - - route = dashboard_override.apply_dashboard_override(facts, "approver") - - self.assertEqual("approver", route) - self.assertTrue(facts["dashboard_override_noop"]) - self.assertTrue(facts["dashboard_override_release_requested"]) - - def test_fresh_command_gets_reply_when_label_already_overrides_author_route(self) -> None: + def test_command_that_cleared_nothing_is_acknowledged_where_it_is_routed(self) -> None: raw = { "issue_comments": [ {"id": 5, "user": {"login": "author"}, "body": "/dashboard route:reviewers"}, ] } - facts = dashboard_override.dashboard_override_facts( - raw, - "author", - {"dashboard:route-overridden"}, - ) + facts = dashboard_override.dashboard_override_facts(raw, "author") - route = dashboard_override.apply_dashboard_override(facts, "author") - dashboard_override.append_route_noop_reply(raw, facts, route) + dashboard_override.append_command_ack_reply(raw, facts, "author") - self.assertEqual("approver", route) - self.assertTrue(facts["dashboard_override_noop"]) self.assertEqual( - [{"comment_id": 5, "kind": "already_routed", "user": "author", "route": "approver"}], + [{"comment_id": 5, "kind": "already_routed", "user": "author", "route": "author"}], facts["dashboard_command_replies"], ) - def test_command_only_overrides_pre_review_routes(self) -> None: - for route, expected_route, expected_pending, expected_noop in ( - ("author", "approver", True, False), - ("approver", "approver", False, True), - ("maintainer", "maintainer", False, True), - ("copilot", "copilot", False, True), - ): - with self.subTest(route=route): - facts = { - "dashboard_override_label_applied": False, - "dashboard_override_requested": True, - "dashboard_override_command_id": 5, - } - - actual = dashboard_override.apply_dashboard_override(facts, route) - - self.assertEqual(expected_route, actual) - self.assertEqual(expected_pending, facts["dashboard_override_requested"]) - self.assertEqual(expected_noop, facts["dashboard_override_noop"]) - - def test_label_yields_to_maintainer_route_and_releases(self) -> None: - facts = { - "dashboard_override_label_applied": True, - "dashboard_override_requested": False, - } - - route = dashboard_override.apply_dashboard_override(facts, "maintainer") - - self.assertEqual("maintainer", route) - self.assertFalse(facts["dashboard_override"]) - self.assertTrue(facts["dashboard_override_release_requested"]) - - def test_releases_label_once_natural_route_reaches_reviewers(self) -> None: - facts = { - "dashboard_override_label_applied": True, - "dashboard_override_requested": False, - } - - route = dashboard_override.apply_dashboard_override(facts, "approver") - - self.assertEqual("approver", route) - self.assertTrue(facts["dashboard_override_release_requested"]) - - def test_does_not_release_label_while_override_holds_author_route(self) -> None: - facts = { - "dashboard_override_label_applied": True, - "dashboard_override_requested": False, - } - - route = dashboard_override.apply_dashboard_override(facts, "author") - - self.assertEqual("approver", route) - self.assertTrue(facts["dashboard_override"]) - self.assertFalse(facts["dashboard_override_release_requested"]) - - @patch.object(dashboard_override, "run_gh") - @patch.object( - dashboard_override, - "load_dashboard_state_cache", - return_value={ - "prs": { - "9": {"facts": {"dashboard_override_release_requested": True}}, - } - }, - ) - def test_delivery_removes_released_override_label(self, _load_state, run_gh) -> None: - errors = dashboard_override.deliver_dashboard_override_requests( - "open-telemetry/example" - ) - - self.assertEqual([], errors) - self.assertEqual( - [ - call([ - "gh", "api", "--method", "DELETE", - "repos/open-telemetry/example/issues/9/labels/dashboard%3Aroute-overridden", - ]), - ], - run_gh.call_args_list, - ) - - @patch.object(dashboard_override, "run_gh") - @patch.object(dashboard_override, "load_dashboard_state_cache", return_value=None) - def test_does_not_create_label_without_dashboard_state(self, _load_state, run_gh) -> None: - errors = dashboard_override.deliver_dashboard_override_requests( - "open-telemetry/example" - ) - - self.assertEqual([], errors) - run_gh.assert_not_called() - - @patch.object(dashboard_override, "run_gh") - @patch.object( - dashboard_override, - "load_dashboard_state_cache", - return_value={"prs": {}}, - ) - def test_does_not_create_label_without_pending_command(self, _load_state, run_gh) -> None: - errors = dashboard_override.deliver_dashboard_override_requests( - "open-telemetry/example" - ) - - self.assertEqual([], errors) - run_gh.assert_not_called() - @patch.object(dashboard_override, "run_gh") @patch.object(dashboard_override, "gh_api", return_value=[]) @patch.object( @@ -721,46 +526,28 @@ def test_does_not_create_label_without_pending_command(self, _load_state, run_gh return_value={ "prs": { "7": { - "route": "copilot", "facts": { - "dashboard_override_requested": True, - "dashboard_override_command_id": 3, - "dashboard_override_command_user": "author", + "dashboard_command_replies": [ + { + "comment_id": 3, + "kind": "routed", + "route": "copilot", + "user": "author", + }, + ] } }, - "8": {"facts": {"dashboard_override_requested": False}}, } }, ) - def test_delivers_pending_override_label_and_acknowledges_command( - self, - _load_state, - gh_api, - run_gh, - ) -> None: - errors = dashboard_override.deliver_dashboard_override_requests( + def test_delivers_command_acknowledgement(self, _load_state, _gh_api, run_gh) -> None: + errors = dashboard_override.deliver_dashboard_command_replies( "open-telemetry/example" ) self.assertEqual([], errors) - gh_api.assert_called_once_with( - "/repos/open-telemetry/example/issues/7/comments?per_page=100", - paginate=True, - ) self.assertEqual( [ - call([ - "gh", "label", "create", "dashboard:route-overridden", - "--repo", "open-telemetry/example", - "--color", dashboard_override.DASHBOARD_OVERRIDE_LABEL_COLOR, - "--description", dashboard_override.DASHBOARD_OVERRIDE_LABEL_DESCRIPTION, - "--force", - ]), - call([ - "gh", "api", "--method", "POST", - "repos/open-telemetry/example/issues/7/labels", - "-f", "labels[]=dashboard:route-overridden", - ]), call([ "gh", "api", "--method", "POST", "repos/open-telemetry/example/issues/7/comments", @@ -770,55 +557,6 @@ def test_delivers_pending_override_label_and_acknowledges_command( run_gh.call_args_list, ) - @patch.object(dashboard_override, "run_gh") - @patch.object(dashboard_override, "gh_api", return_value=[]) - @patch.object( - dashboard_override, - "load_dashboard_state_cache", - return_value={ - "prs": { - "7": { - "facts": { - "dashboard_override_requested": True, - "dashboard_override_command_id": 3, - "dashboard_override_command_user": "author", - } - }, - } - }, - ) - def test_retry_observes_acknowledgement_after_ambiguous_post_failure( - self, - _load_state, - gh_api, - run_gh, - ) -> None: - run_gh.side_effect = [None, None, RuntimeError("comment response lost")] - - errors = dashboard_override.deliver_dashboard_override_requests( - "open-telemetry/example" - ) - - self.assertEqual(["PR #7: comment response lost"], errors) - - run_gh.reset_mock() - run_gh.side_effect = None - gh_api.return_value = [{ - "performed_via_github_app": {"slug": "opentelemetry-pr-dashboard"}, - "body": dashboard_override.render_command_reply({ - "comment_id": 3, - "kind": "routed", - "user": "author", - }), - }] - - errors = dashboard_override.deliver_dashboard_override_requests( - "open-telemetry/example" - ) - - self.assertEqual([], errors) - self.assertEqual(2, run_gh.call_count) - if __name__ == "__main__": unittest.main() \ No newline at end of file diff --git a/.github/scripts/pull-request-dashboard/test_delivery.py b/.github/scripts/pull-request-dashboard/test_delivery.py index eb58bbd9e4c..acf0122c62d 100644 --- a/.github/scripts/pull-request-dashboard/test_delivery.py +++ b/.github/scripts/pull-request-dashboard/test_delivery.py @@ -13,7 +13,6 @@ class DeliveryTest(unittest.TestCase): @patch.object(delivery, "deliver_copilot_review_requests", return_value=[]) @patch.object(delivery, "deliver_prepared_author_nudges", return_value=[]) @patch.object(delivery, "deliver_dashboard_command_replies", return_value=[]) - @patch.object(delivery, "deliver_dashboard_override_requests", return_value=[]) @patch.object(delivery, "update_status_comments_from_state", return_value=[]) @patch.object( delivery, @@ -27,7 +26,6 @@ def test_runs_all_repository_deliveries_in_order( self, _list_open, status_comments, - dashboard_overrides, dashboard_command_replies, author_nudges, copilot_reviews, @@ -40,7 +38,6 @@ def record(label: str) -> list[str]: return [] status_comments.side_effect = lambda *_args: record("status") - dashboard_overrides.side_effect = lambda *_args: record("override") dashboard_command_replies.side_effect = lambda *_args: record("replies") author_nudges.side_effect = lambda *_args: record("author") copilot_reviews.side_effect = lambda *_args: record("copilot") @@ -55,7 +52,7 @@ def record(label: str) -> list[str]: self.assertEqual([], errors) _list_open.assert_called_once_with("open-telemetry/example") self.assertEqual( - [call("override"), call("replies"), call("author"), call("status"), call("copilot"), call("slack")], + [call("replies"), call("author"), call("status"), call("copilot"), call("slack")], order.call_args_list, ) status_comments.assert_called_once_with( @@ -76,7 +73,6 @@ def record(label: str) -> list[str]: @patch.object(delivery, "deliver_copilot_review_requests", return_value=[]) @patch.object(delivery, "deliver_prepared_author_nudges", return_value=[]) @patch.object(delivery, "deliver_dashboard_command_replies", return_value=[]) - @patch.object(delivery, "deliver_dashboard_override_requests", return_value=[]) @patch.object(delivery, "update_status_comments_from_state", side_effect=RuntimeError("boom")) @patch.object( delivery, @@ -87,7 +83,6 @@ def test_failure_does_not_block_later_deliveries( self, _list_open, _status_comments, - dashboard_overrides, dashboard_command_replies, author_nudges, copilot_reviews, @@ -101,7 +96,6 @@ def test_failure_does_not_block_later_deliveries( ) self.assertIn("status comments: boom", errors) - dashboard_overrides.assert_called_once() dashboard_command_replies.assert_called_once() author_nudges.assert_called_once() copilot_reviews.assert_called_once() @@ -110,7 +104,6 @@ def test_failure_does_not_block_later_deliveries( def test_open_pr_list_failure_skips_dependent_stages(self) -> None: with ( patch.object(delivery, "list_open_prs", side_effect=RuntimeError("unavailable")), - patch.object(delivery, "deliver_dashboard_override_requests", return_value=[]) as overrides, patch.object(delivery, "deliver_dashboard_command_replies", return_value=[]) as replies, patch.object(delivery, "deliver_prepared_author_nudges", return_value=[]) as nudges, patch.object(delivery, "update_status_comments_from_state", return_value=[]) as status, @@ -125,7 +118,6 @@ def test_open_pr_list_failure_skips_dependent_stages(self) -> None: ) self.assertEqual(["open pull requests: unavailable"], errors) - overrides.assert_called_once() replies.assert_called_once() nudges.assert_called_once() copilot.assert_called_once() @@ -140,7 +132,6 @@ def test_targeted_delivery_only_processes_triggering_pr(self) -> None: "gh_api", return_value={"state": "open", "draft": False, "title": "Seven"}, ) as gh_api, - patch.object(delivery, "deliver_dashboard_override_requests", return_value=[]), patch.object(delivery, "deliver_dashboard_command_replies", return_value=[]), patch.object(delivery, "deliver_prepared_author_nudges", return_value=[]), patch.object(delivery, "update_status_comments_from_state") as bulk_status, diff --git a/.github/scripts/pull-request-dashboard/test_render.py b/.github/scripts/pull-request-dashboard/test_render.py index 17a913aa73c..09d30b6df29 100644 --- a/.github/scripts/pull-request-dashboard/test_render.py +++ b/.github/scripts/pull-request-dashboard/test_render.py @@ -211,26 +211,6 @@ def test_omits_labels_when_none_are_configured(self) -> None: ) self.assertNotIn("", render_pr_tables(prs, results)) - def test_renders_route_override_label_without_configuration(self) -> None: - prs = [ - { - "number": 127, - "title": "Feature", - "author": {"login": "author"}, - "isDraft": False, - "labels": ["size/L", "dashboard:route-overridden"], - }, - ] - results = {127: {"route": "reviewers", "facts": {}}} - - markdown = render_pr_tables(prs, results) - - self.assertIn( - "#127 Feature · dashboard:route-overridden", - markdown, - ) - self.assertNotIn("size/L", markdown) - if __name__ == "__main__": unittest.main() diff --git a/pull-request-dashboard/README.md b/pull-request-dashboard/README.md index 8ab03594b61..8f7b6a6c939 100644 --- a/pull-request-dashboard/README.md +++ b/pull-request-dashboard/README.md @@ -12,7 +12,7 @@ The classification cache reuses prior results for unchanged review threads, mini The dashboard groups open non-draft pull requests by who is expected to act next (e.g. *Waiting on reviewers*, *Waiting on authors*, *Waiting on maintainers*). Draft PRs are listed separately at the bottom unless `large_repo` rendering is enabled. Within each group, rows are sorted longest-waiting first. Every row has these six columns: -- **PR** — Pull request number and title, followed by any configured matching labels. The number autolinks to the PR on GitHub. Configured labels are rendered inline for both active and draft PRs. The `dashboard:route-overridden` label is always rendered, regardless of configuration. +- **PR** — Pull request number and title, followed by any configured matching labels. The number autolinks to the PR on GitHub. Configured labels are rendered inline for both active and draft PRs. - **Author** — GitHub login of the PR author. - **Reviewers** — Reviewers who have engaged with the PR, each annotated with one or more icons: - ✅ approved @@ -69,7 +69,7 @@ Fields: | `name` | yes | Name of the repository under `open-telemetry`. | | `approver_teams` | yes | GitHub team slugs whose members count as approvers. | | `required_approvals` | no | Number of approvals required for an open PR to be marked ready to merge. Defaults to `1`. | -| `labels_to_display` | no | Case-sensitive shell-style label name patterns to display inline after PR titles. Exact names such as `breaking change` and wildcard patterns such as `size/*` are supported. Defaults to `[]`, which displays only the `dashboard:route-overridden` label. | +| `labels_to_display` | no | Case-sensitive shell-style label name patterns to display inline after PR titles. Exact names such as `breaking change` and wildcard patterns such as `size/*` are supported. Defaults to `[]`, which displays no labels. | | `non_blocking_check_patterns` | no | Check-name globs for non-required checks whose failures should be identified in the live PR status comment. When the PR is waiting on the author, matching failures are reported only when at least one required check is failing and are noted alongside those failures. On other routes, matching failures are shown separately. Matching checks remain informational and do not affect routing or the dashboard CI column. | | `require_clean_copilot_review_branches` | no | List of base branch names for which a Copilot review with no inline findings on the current head is required before routing a PR to reviewers or maintainers. The dashboard re-requests Copilot review when needed and does not duplicate a pending request. List only branches where automatic Copilot code review is enabled (typically `["main"]`); PRs targeting any other branch are never gated, so they cannot stall waiting for a review that never runs. Defaults to `[]` (no branches gated). | | `slack_channel` | no | Slack channel for notifications. Omit to skip Slack processing for this repository. | @@ -186,24 +186,19 @@ believes it is ready for another review, the author can comment `/dashboard route:reviewers`. The command clears every review item and required check failure that was already open when it was posted, so the pull request routes to *Waiting on reviewers* and stays there instead of falling -back to the author on the next refresh. The dashboard applies the -`dashboard:route-overridden` label to -mark the override, which the dashboard shows inline after the PR title. Members of the repository's `approver_teams` can use the same +back to the author on the next refresh. Members of the repository's +`approver_teams` can use the same command. A `/dashboard route:reviewers` command from anyone else, or a command -on a pull request already at or past reviewers, has no routing effect. The +that clears nothing, has no routing effect. The dashboard replies to a `/dashboard route:reviewers` from an unauthorized user explaining that only the author or an approver can use it, replies to an author -or approver command on a pull request already at or past reviewers noting where -it is currently routed, and replies to any unrecognized `/dashboard` command. +or approver command that cleared nothing noting where it is currently routed, +and replies to any unrecognized `/dashboard` command. Anything that happens after the command keeps its author action, so new review feedback and new check failures still route the pull request back to the author. -Removing the `dashboard:route-overridden` label restores automatic routing for -the cleared items too. The dashboard also removes the label automatically once -the override no longer clears anything, so a forgotten override does not linger. -A command that has already been applied is not replayed after label removal; the -author can post a new `/dashboard route:reviewers` command if another override -is needed later. +A reviewer who disagrees that a cleared item was handled says so on that item, +which routes the pull request back to the author on the next refresh. ## Author reminder From fb9245fbdb1a7d2b49d7b63a7ffca71a93a008a9 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Thu, 30 Jul 2026 13:52:40 -0700 Subject: [PATCH 03/11] Address review comment from Copilot: rename the override-cleared CI test helper --- .../scripts/pull-request-dashboard/test_dashboard.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/scripts/pull-request-dashboard/test_dashboard.py b/.github/scripts/pull-request-dashboard/test_dashboard.py index 033011af3e0..8353509854f 100644 --- a/.github/scripts/pull-request-dashboard/test_dashboard.py +++ b/.github/scripts/pull-request-dashboard/test_dashboard.py @@ -31,9 +31,9 @@ class ResolvePrRouteTest(unittest.TestCase): - def _author_route_facts(self, **overrides: object) -> dict[str, object]: - # A failing required check routes a human-authored PR to the author - # unless an override already cleared it. + def _cleared_ci_facts(self, **overrides: object) -> dict[str, object]: + # A failing required check that an override already cleared, which + # would otherwise route a human-authored pull request to the author. facts: dict[str, object] = { "ci_failing_count": 1, "dashboard_override_cleared_ci": True, @@ -42,7 +42,7 @@ def _author_route_facts(self, **overrides: object) -> dict[str, object]: return facts def test_override_is_still_gated_by_required_copilot_review(self) -> None: - facts = self._author_route_facts( + facts = self._cleared_ci_facts( copilot_review_exists=True, copilot_review_needed=True, copilot_review_requested=False, @@ -53,7 +53,7 @@ def test_override_is_still_gated_by_required_copilot_review(self) -> None: self.assertEqual("copilot", route) def test_override_reaches_reviewers_when_copilot_review_is_clean(self) -> None: - facts = self._author_route_facts( + facts = self._cleared_ci_facts( copilot_review_exists=True, copilot_review_needed=False, ) @@ -63,7 +63,7 @@ def test_override_reaches_reviewers_when_copilot_review_is_clean(self) -> None: self.assertEqual("approver", route) def test_override_reaches_reviewers_when_gate_disabled(self) -> None: - facts = self._author_route_facts() + facts = self._cleared_ci_facts() route = resolve_pr_route(facts, {}, 1, False) From 8a673418bea4cb2372a2e9f8a4207fca6b200a8b Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Thu, 30 Jul 2026 14:26:51 -0700 Subject: [PATCH 04/11] Address review comment from Copilot: compare override watermarks as parsed timestamps --- .../dashboard_override.py | 12 +++++++----- .../test_dashboard_override.py | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/.github/scripts/pull-request-dashboard/dashboard_override.py b/.github/scripts/pull-request-dashboard/dashboard_override.py index 822edac3f68..e0643a18385 100644 --- a/.github/scripts/pull-request-dashboard/dashboard_override.py +++ b/.github/scripts/pull-request-dashboard/dashboard_override.py @@ -7,7 +7,7 @@ from github_cli import gh_api, run_gh from state import load_dashboard_state_cache -from utils import actor_login +from utils import actor_login, parse_ts DASHBOARD_COMMAND_PREFIX = "/dashboard" @@ -351,20 +351,22 @@ def clear_overridden_actions( reaches the author, including a reviewer reopening something the command cleared. """ - override_since = facts.get("dashboard_override_since") or "" + # Timestamps reach here in both GitHub's `...Z` form and `format_ts`'s + # `...+00:00` form, so they have to be parsed rather than compared as text. + override_since = parse_ts(facts.get("dashboard_override_since")) facts["dashboard_override_cleared_count"] = 0 facts["dashboard_override_cleared_ci"] = False - if not override_since: + if override_since is None: return pending_actions remaining: dict[str, dict[str, Any]] = {} cleared = 0 for discussion_id, entry in pending_actions.items(): - since = entry.get("since") or "" + since = parse_ts(entry.get("since")) if entry.get("action") == "author" and since and since <= override_since: cleared += 1 continue remaining[discussion_id] = entry - ci_failing_since = facts.get("ci_failing_since") or "" + ci_failing_since = parse_ts(facts.get("ci_failing_since")) facts["dashboard_override_cleared_count"] = cleared facts["dashboard_override_cleared_ci"] = bool( facts.get("ci_failing_count") diff --git a/.github/scripts/pull-request-dashboard/test_dashboard_override.py b/.github/scripts/pull-request-dashboard/test_dashboard_override.py index 239648bd1f1..a7336fa01d6 100644 --- a/.github/scripts/pull-request-dashboard/test_dashboard_override.py +++ b/.github/scripts/pull-request-dashboard/test_dashboard_override.py @@ -494,6 +494,25 @@ def test_clears_check_failures_that_predate_the_command(self) -> None: self.assertEqual(expected, facts["dashboard_override_cleared_ci"]) + def test_clears_items_that_share_the_command_timestamp(self) -> None: + # The command's own `...Z` timestamp is compared against `format_ts` + # output, so the same instant reaches here written two different ways. + facts = { + "dashboard_override_since": "2026-07-30T12:00:00Z", + "ci_failing_count": 1, + "ci_failing_since": "2026-07-30T12:00:00+00:00", + } + pending_actions = { + "same": {"action": "author", "since": "2026-07-30T12:00:00+00:00"}, + "later": {"action": "author", "since": "2026-07-30T12:00:00.500000+00:00"}, + } + + remaining = dashboard_override.clear_overridden_actions(facts, pending_actions) + + self.assertEqual(["later"], sorted(remaining)) + self.assertEqual(1, facts["dashboard_override_cleared_count"]) + self.assertTrue(facts["dashboard_override_cleared_ci"]) + def test_clears_nothing_without_a_command(self) -> None: facts = {"dashboard_override_since": ""} pending_actions = {"old": {"action": "author", "since": "2026-07-30T11:00:00Z"}} From e4f78a6613404364605f9c17c7b9f16696c4c08d Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Thu, 30 Jul 2026 14:44:50 -0700 Subject: [PATCH 05/11] Address review comments from Copilot: key cleared CI off the newest failure and share the uncleared count --- .../pull-request-dashboard/dashboard.py | 8 ++++--- .../dashboard_override.py | 15 ++++++++++--- .../pr_status_comment.py | 4 ++-- .../test_dashboard_override.py | 22 +++++++++++++++---- .../test_pr_status_comment.py | 14 ++++++++++++ 5 files changed, 51 insertions(+), 12 deletions(-) diff --git a/.github/scripts/pull-request-dashboard/dashboard.py b/.github/scripts/pull-request-dashboard/dashboard.py index 4c1b6387a45..376914b11a1 100644 --- a/.github/scripts/pull-request-dashboard/dashboard.py +++ b/.github/scripts/pull-request-dashboard/dashboard.py @@ -109,6 +109,8 @@ fetched. ci_failing_since str (iso) Earliest completion time among current required failures. + ci_failing_latest str (iso) Latest completion time among + current required failures. ci_pending_count int Merge-blocking checks only; absent when checks could not be fetched. @@ -200,6 +202,7 @@ clear_overridden_actions, dashboard_command_body_remainder, dashboard_override_facts, + uncleared_ci_failing_count, ) from pr_status_comment import status_author_nudge_episode_id from state import ( @@ -567,6 +570,7 @@ def compute_facts( facts["ci_failing_count"] = len(failing) if failing_timestamps: facts["ci_failing_since"] = format_ts(min(failing_timestamps)) + facts["ci_failing_latest"] = format_ts(max(failing_timestamps)) facts["ci_pending_count"] = len(pending) non_blocking_check_failures = sorted({ check.get("name") or "" @@ -1114,9 +1118,7 @@ def route_pr(facts: dict[str, Any], pending_actions: dict[str, dict[str, Any]], # 3. If there are enough approvals and no inline or top-level feedback is # still waiting on a reviewer -> "maintainer". # 4. Otherwise the PR is still waiting on approvers. - ci_failing = facts.get("ci_failing_count", 0) > 0 and not facts.get( - "dashboard_override_cleared_ci" - ) + ci_failing = uncleared_ci_failing_count(facts) > 0 if ci_failing and not is_maintenance_bot: return "author" if counts["author"] and not is_maintenance_bot: diff --git a/.github/scripts/pull-request-dashboard/dashboard_override.py b/.github/scripts/pull-request-dashboard/dashboard_override.py index e0643a18385..ea35850a70d 100644 --- a/.github/scripts/pull-request-dashboard/dashboard_override.py +++ b/.github/scripts/pull-request-dashboard/dashboard_override.py @@ -366,16 +366,25 @@ def clear_overridden_actions( cleared += 1 continue remaining[discussion_id] = entry - ci_failing_since = parse_ts(facts.get("ci_failing_since")) + ci_failing_latest = parse_ts(facts.get("ci_failing_latest")) facts["dashboard_override_cleared_count"] = cleared + # Keyed off the newest failure so a check that fails after the command still + # routes to the author, even while an older cleared failure persists. facts["dashboard_override_cleared_ci"] = bool( facts.get("ci_failing_count") - and ci_failing_since - and ci_failing_since <= override_since + and ci_failing_latest + and ci_failing_latest <= override_since ) return remaining +def uncleared_ci_failing_count(facts: dict[str, Any]) -> int: + """Failing required checks that an override command has not cleared.""" + if facts.get("dashboard_override_cleared_ci"): + return 0 + return facts.get("ci_failing_count") or 0 + + def append_command_ack_reply( raw: dict[str, Any], facts: dict[str, Any], diff --git a/.github/scripts/pull-request-dashboard/pr_status_comment.py b/.github/scripts/pull-request-dashboard/pr_status_comment.py index 96220394788..486aa56d7dc 100644 --- a/.github/scripts/pull-request-dashboard/pr_status_comment.py +++ b/.github/scripts/pull-request-dashboard/pr_status_comment.py @@ -12,7 +12,7 @@ gh_api, run_gh, ) -from dashboard_override import PRE_REVIEW_ROUTES +from dashboard_override import PRE_REVIEW_ROUTES, uncleared_ci_failing_count from route_presentation import route_status_summary, status_headline from state import ( STATUS_COMMENT_REVISION, @@ -221,7 +221,7 @@ def render_status_comment( review_thread_urls = facts.get("author_action_review_thread_urls") or [] top_level_feedback_urls = facts.get("author_action_top_level_feedback_urls") or [] feedback_count = len(review_thread_urls) + len(top_level_feedback_urls) - failing_count = facts.get("ci_failing_count", 0) + failing_count = uncleared_ci_failing_count(facts) non_blocking_check_failures = facts.get("non_blocking_check_failures") or [] override_route = "" diff --git a/.github/scripts/pull-request-dashboard/test_dashboard_override.py b/.github/scripts/pull-request-dashboard/test_dashboard_override.py index a7336fa01d6..f403262b02e 100644 --- a/.github/scripts/pull-request-dashboard/test_dashboard_override.py +++ b/.github/scripts/pull-request-dashboard/test_dashboard_override.py @@ -479,28 +479,42 @@ def test_clears_only_author_actions_that_predate_the_command(self) -> None: self.assertEqual(1, facts["dashboard_override_cleared_count"]) def test_clears_check_failures_that_predate_the_command(self) -> None: - for ci_failing_since, expected in ( + for ci_failing_latest, expected in ( ("2026-07-30T11:00:00Z", True), ("2026-07-30T13:00:00Z", False), ): - with self.subTest(ci_failing_since=ci_failing_since): + with self.subTest(ci_failing_latest=ci_failing_latest): facts = { "dashboard_override_since": "2026-07-30T12:00:00Z", "ci_failing_count": 1, - "ci_failing_since": ci_failing_since, + "ci_failing_since": ci_failing_latest, + "ci_failing_latest": ci_failing_latest, } dashboard_override.clear_overridden_actions(facts, {}) self.assertEqual(expected, facts["dashboard_override_cleared_ci"]) + def test_keeps_a_check_that_fails_after_the_command(self) -> None: + facts = { + "dashboard_override_since": "2026-07-30T12:00:00Z", + "ci_failing_count": 2, + "ci_failing_since": "2026-07-30T11:00:00Z", + "ci_failing_latest": "2026-07-30T13:00:00Z", + } + + dashboard_override.clear_overridden_actions(facts, {}) + + self.assertFalse(facts["dashboard_override_cleared_ci"]) + self.assertEqual(2, dashboard_override.uncleared_ci_failing_count(facts)) + def test_clears_items_that_share_the_command_timestamp(self) -> None: # The command's own `...Z` timestamp is compared against `format_ts` # output, so the same instant reaches here written two different ways. facts = { "dashboard_override_since": "2026-07-30T12:00:00Z", "ci_failing_count": 1, - "ci_failing_since": "2026-07-30T12:00:00+00:00", + "ci_failing_latest": "2026-07-30T12:00:00+00:00", } pending_actions = { "same": {"action": "author", "since": "2026-07-30T12:00:00+00:00"}, 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 0860169573d..76670c4a5e7 100644 --- a/.github/scripts/pull-request-dashboard/test_pr_status_comment.py +++ b/.github/scripts/pull-request-dashboard/test_pr_status_comment.py @@ -378,6 +378,20 @@ def test_non_author_routes_also_name_required_ci_failures(self) -> None: self.assertIn(f"**Also blocked by:** {blocked_by}", body) self.assertIn(non_blocking_line, body.splitlines()) + def test_override_cleared_check_failure_is_not_reported_as_blocking(self) -> None: + body = pr_status_comment.render_status_comment( + self.pr(), + { + "route": "approver", + "facts": { + "ci_failing_count": 1, + "dashboard_override_cleared_ci": True, + }, + }, + ) + + self.assertNotIn("**Also blocked by:**", body) + def test_waiting_on_author_caps_feedback_links_across_sections(self) -> None: review_thread_urls = [ f"https://github.com/open-telemetry/example/pull/1#discussion_r{index}" From 2d306529cb9c66d5410c295814ca7aa1b803cd0c Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Thu, 30 Jul 2026 14:48:06 -0700 Subject: [PATCH 06/11] Address review comment from Copilot: keep acknowledged override commands authorized --- .../dashboard_override.py | 32 +++++++++++---- .../test_dashboard_override.py | 39 ++++++++++++++++++- 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/.github/scripts/pull-request-dashboard/dashboard_override.py b/.github/scripts/pull-request-dashboard/dashboard_override.py index ea35850a70d..b0a941dcd36 100644 --- a/.github/scripts/pull-request-dashboard/dashboard_override.py +++ b/.github/scripts/pull-request-dashboard/dashboard_override.py @@ -109,13 +109,20 @@ def latest_authorized_command_at( Unlike `latest_authorized_command`, acknowledged commands still count: the watermark has to outlive the acknowledgement, or the discussions a command - cleared would come back on the next refresh. + cleared would come back on the next refresh. An acknowledgement is also + treated as durable proof of authorization, since approver-team membership is + resolved live and an approver can leave the team after commanding. """ + acknowledged_ids = _acknowledged_override_command_ids(raw.get("issue_comments")) latest = "" for comment in raw.get("issue_comments") or []: if parse_dashboard_command(comment) != DASHBOARD_OVERRIDE_SUBCOMMAND: continue - if not is_authorized_commander( + try: + comment_id = int(comment.get("id")) + except (TypeError, ValueError): + comment_id = 0 + if comment_id not in acknowledged_ids and not is_authorized_commander( actor_login(comment.get("user") or {}), author, reviewers ): continue @@ -168,16 +175,27 @@ def _replied_command_ids(comments: list[dict[str, Any]] | None) -> set[int]: return replied_ids -def _acknowledged_override_command_id( +def _acknowledged_override_command_ids( comments: list[dict[str, Any]] | None, -) -> int: - acknowledged_id = 0 +) -> set[int]: + """Command ids the dashboard app has already acknowledged. + + Ack markers are matched only in comments authored by the dashboard app, so a + user cannot forge one to authorize their own command. + """ + acknowledged_ids: set[int] = set() for comment in comments or []: if not _is_dashboard_app_comment(comment): continue for match in _OVERRIDE_ACK_MARKER_RE.findall(comment.get("body") or ""): - acknowledged_id = max(acknowledged_id, int(match)) - return acknowledged_id + acknowledged_ids.add(int(match)) + return acknowledged_ids + + +def _acknowledged_override_command_id( + comments: list[dict[str, Any]] | None, +) -> int: + return max(_acknowledged_override_command_ids(comments), default=0) def pending_command_replies( diff --git a/.github/scripts/pull-request-dashboard/test_dashboard_override.py b/.github/scripts/pull-request-dashboard/test_dashboard_override.py index f403262b02e..7af95fcdc34 100644 --- a/.github/scripts/pull-request-dashboard/test_dashboard_override.py +++ b/.github/scripts/pull-request-dashboard/test_dashboard_override.py @@ -465,7 +465,44 @@ def test_watermark_survives_acknowledgement(self) -> None: self.assertEqual(0, facts["dashboard_override_command_id"]) self.assertEqual("2026-07-30T12:00:00Z", facts["dashboard_override_since"]) - def test_clears_only_author_actions_that_predate_the_command(self) -> None: + def test_watermark_survives_the_commander_leaving_the_approver_team(self) -> None: + raw = { + "issue_comments": [ + { + "id": 3, + "user": {"login": "former-approver"}, + "created_at": "2026-07-30T12:00:00Z", + "body": "/dashboard route:reviewers", + }, + { + "id": 4, + "user": {"login": "opentelemetry-pr-dashboard[bot]"}, + "created_at": "2026-07-30T12:05:00Z", + "body": dashboard_override.override_ack_marker(3), + }, + ] + } + + facts = dashboard_override.dashboard_override_facts(raw, "author", set()) + + self.assertEqual("2026-07-30T12:00:00Z", facts["dashboard_override_since"]) + + def test_unacknowledged_command_needs_current_authorization(self) -> None: + raw = { + "issue_comments": [ + { + "id": 3, + "user": {"login": "former-approver"}, + "created_at": "2026-07-30T12:00:00Z", + "body": "/dashboard route:reviewers", + }, + ] + } + + facts = dashboard_override.dashboard_override_facts(raw, "author", set()) + + self.assertEqual("", facts["dashboard_override_since"]) + facts = {"dashboard_override_since": "2026-07-30T12:00:00Z"} pending_actions = { "old": {"action": "author", "since": "2026-07-30T11:00:00Z"}, From 0347d84e01f8a7b6d6b31903fcb10a1cf0bd77bd Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Thu, 30 Jul 2026 14:59:07 -0700 Subject: [PATCH 07/11] Address review comment from Copilot: acknowledge the maintainer route in the command reply --- .../pull-request-dashboard/dashboard_override.py | 5 +++++ .../test_dashboard_override.py | 12 ++++++++++++ 2 files changed, 17 insertions(+) diff --git a/.github/scripts/pull-request-dashboard/dashboard_override.py b/.github/scripts/pull-request-dashboard/dashboard_override.py index b0a941dcd36..97a0fad9864 100644 --- a/.github/scripts/pull-request-dashboard/dashboard_override.py +++ b/.github/scripts/pull-request-dashboard/dashboard_override.py @@ -286,6 +286,11 @@ def render_command_reply(reply: dict[str, Any]) -> str: "accepted the reviewer-routing override; the reviewer handoff " "is waiting on Copilot." ) + elif route == "maintainer": + message = ( + "accepted the reviewer-routing override; this pull request has " + "the approvals it needs and is now waiting on maintainers." + ) else: message = "routed this pull request to reviewers." else: diff --git a/.github/scripts/pull-request-dashboard/test_dashboard_override.py b/.github/scripts/pull-request-dashboard/test_dashboard_override.py index 7af95fcdc34..e47534beb0f 100644 --- a/.github/scripts/pull-request-dashboard/test_dashboard_override.py +++ b/.github/scripts/pull-request-dashboard/test_dashboard_override.py @@ -156,6 +156,12 @@ def test_renders_command_replies(self) -> None: "route": "copilot", "user": "author", }) + maintainer = dashboard_override.render_command_reply({ + "comment_id": 6, + "kind": "routed", + "route": "maintainer", + "user": "author", + }) self.assertIn(dashboard_override.command_reply_marker(2), unauthorized) self.assertIn( @@ -177,6 +183,12 @@ def test_renders_command_replies(self) -> None: "handoff is waiting on Copilot.", copilot_gated, ) + self.assertIn(dashboard_override.command_reply_marker(6), maintainer) + self.assertIn( + "@author accepted the reviewer-routing override; this pull request " + "has the approvals it needs and is now waiting on maintainers.", + maintainer, + ) def test_renders_already_routed_replies_per_route(self) -> None: cases = { From c84e7754a27a6fb920099ca5acda85f09b0990f0 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Thu, 30 Jul 2026 15:08:52 -0700 Subject: [PATCH 08/11] Address review comments from Copilot: bump the dashboard state version and status comment revision --- .github/scripts/pull-request-dashboard/state.py | 4 ++-- .github/scripts/pull-request-dashboard/test_state.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/scripts/pull-request-dashboard/state.py b/.github/scripts/pull-request-dashboard/state.py index d386fd59e06..25e483884d3 100644 --- a/.github/scripts/pull-request-dashboard/state.py +++ b/.github/scripts/pull-request-dashboard/state.py @@ -23,7 +23,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 = 6 +DASHBOARD_STATE_VERSION = 7 # backfill-state.json: round-robin cursor used by full dashboard refreshes. BACKFILL_STATE_VERSION = 3 # notification-state.json: pending and delivered Slack notification records. @@ -36,7 +36,7 @@ STATUS_COMMENT_ROLLOUT_STATE_VERSION = 1 # 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 = 14 +STATUS_COMMENT_REVISION = 15 INITIAL_BACKFILL_COMPLETE_KEY = "initial_backfill_complete" _state_dir: Path | None = None diff --git a/.github/scripts/pull-request-dashboard/test_state.py b/.github/scripts/pull-request-dashboard/test_state.py index c94ae35901c..d78dd9fca01 100644 --- a/.github/scripts/pull-request-dashboard/test_state.py +++ b/.github/scripts/pull-request-dashboard/test_state.py @@ -164,7 +164,7 @@ def test_dashboard_state_save_writes_explicit_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, 6) + self.assertEqual(DASHBOARD_STATE_VERSION, 7) self.assertEqual(STATUS_COMMENT_ROLLOUT_STATE_VERSION, 1) self.assertEqual(AUTHOR_NUDGE_STATE_VERSION, 2) self.assertEqual(COPILOT_REVIEW_REQUEST_STATE_VERSION, 3) From e0a0f6bff8b98b88dfca49bf7bad69b745ad2f9d Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Thu, 30 Jul 2026 15:21:43 -0700 Subject: [PATCH 09/11] Address review comment(s) from Copilot: clear only the check failures that predate the override command --- .../pull-request-dashboard/dashboard.py | 23 +++++++-- .../dashboard_override.py | 14 ++---- .../pull-request-dashboard/test_dashboard.py | 47 +++++++++++++++++-- .../test_dashboard_override.py | 19 +++----- .../test_pr_status_comment.py | 16 +++---- 5 files changed, 78 insertions(+), 41 deletions(-) diff --git a/.github/scripts/pull-request-dashboard/dashboard.py b/.github/scripts/pull-request-dashboard/dashboard.py index 376914b11a1..7a46ccbad1e 100644 --- a/.github/scripts/pull-request-dashboard/dashboard.py +++ b/.github/scripts/pull-request-dashboard/dashboard.py @@ -109,8 +109,10 @@ fetched. ci_failing_since str (iso) Earliest completion time among current required failures. - ci_failing_latest str (iso) Latest completion time among - current required failures. + ci_uncleared_failing_count int Required failures an override + command has not cleared. + ci_uncleared_failing_since str (iso) Earliest completion time among + those uncleared failures. ci_pending_count int Merge-blocking checks only; absent when checks could not be fetched. @@ -570,7 +572,18 @@ def compute_facts( facts["ci_failing_count"] = len(failing) if failing_timestamps: facts["ci_failing_since"] = format_ts(min(failing_timestamps)) - facts["ci_failing_latest"] = format_ts(max(failing_timestamps)) + # A failure with no completion time cannot be shown to predate the + # override command, so it counts as uncleared. + untimed = len(failing) - len(failing_timestamps) + override_since = parse_ts(facts.get("dashboard_override_since") or "") + uncleared = [ + ts + for ts in failing_timestamps + if override_since is None or ts > override_since + ] + facts["ci_uncleared_failing_count"] = untimed + len(uncleared) + if uncleared: + facts["ci_uncleared_failing_since"] = format_ts(min(uncleared)) facts["ci_pending_count"] = len(pending) non_blocking_check_failures = sorted({ check.get("name") or "" @@ -1149,8 +1162,8 @@ def fallback_wait_ts(route: str, facts: dict[str, Any]) -> tuple[datetime | None if route in ("approver", "maintainer", "copilot"): return parse_ts(facts.get("last_author_activity_at") or ""), "last_author_activity" if route == "author": - if facts.get("ci_failing_count", 0) > 0: - ci_failing_since = parse_ts(facts.get("ci_failing_since") or "") + if uncleared_ci_failing_count(facts) > 0: + ci_failing_since = parse_ts(facts.get("ci_uncleared_failing_since") or "") if ci_failing_since is not None: return ci_failing_since, "ci_failure" return parse_ts(facts.get("last_author_activity_at") or ""), "last_author_activity" diff --git a/.github/scripts/pull-request-dashboard/dashboard_override.py b/.github/scripts/pull-request-dashboard/dashboard_override.py index 97a0fad9864..f4217479120 100644 --- a/.github/scripts/pull-request-dashboard/dashboard_override.py +++ b/.github/scripts/pull-request-dashboard/dashboard_override.py @@ -389,23 +389,15 @@ def clear_overridden_actions( cleared += 1 continue remaining[discussion_id] = entry - ci_failing_latest = parse_ts(facts.get("ci_failing_latest")) + ci_failing_count = facts.get("ci_failing_count") or 0 facts["dashboard_override_cleared_count"] = cleared - # Keyed off the newest failure so a check that fails after the command still - # routes to the author, even while an older cleared failure persists. - facts["dashboard_override_cleared_ci"] = bool( - facts.get("ci_failing_count") - and ci_failing_latest - and ci_failing_latest <= override_since - ) + facts["dashboard_override_cleared_ci"] = uncleared_ci_failing_count(facts) < ci_failing_count return remaining def uncleared_ci_failing_count(facts: dict[str, Any]) -> int: """Failing required checks that an override command has not cleared.""" - if facts.get("dashboard_override_cleared_ci"): - return 0 - return facts.get("ci_failing_count") or 0 + return facts.get("ci_uncleared_failing_count") or 0 def append_command_ack_reply( diff --git a/.github/scripts/pull-request-dashboard/test_dashboard.py b/.github/scripts/pull-request-dashboard/test_dashboard.py index 8353509854f..83dc7187bac 100644 --- a/.github/scripts/pull-request-dashboard/test_dashboard.py +++ b/.github/scripts/pull-request-dashboard/test_dashboard.py @@ -36,7 +36,7 @@ def _cleared_ci_facts(self, **overrides: object) -> dict[str, object]: # would otherwise route a human-authored pull request to the author. facts: dict[str, object] = { "ci_failing_count": 1, - "dashboard_override_cleared_ci": True, + "ci_uncleared_failing_count": 0, } facts.update(overrides) return facts @@ -890,7 +890,7 @@ def test_required_check_buckets_control_ci_facts_and_author_routing(self) -> Non def test_required_ci_failure_routes_to_author_before_approval_state(self) -> None: facts = { "approval_count": 1, - "ci_failing_count": 1, + "ci_uncleared_failing_count": 1, "is_maintenance_bot": False, } @@ -900,8 +900,8 @@ def test_override_cleared_ci_failure_does_not_route_to_author(self) -> None: facts = { "approval_count": 0, "ci_failing_count": 1, + "ci_uncleared_failing_count": 0, "is_maintenance_bot": False, - "dashboard_override_cleared_ci": True, } self.assertEqual("approver", route_pr(facts, {}, 1)) @@ -911,7 +911,7 @@ def test_required_ci_failure_preserves_maintenance_bot_routing(self) -> None: with self.subTest(approval_count=approval_count): facts = { "approval_count": approval_count, - "ci_failing_count": 1, + "ci_uncleared_failing_count": 1, "is_maintenance_bot": True, } @@ -966,6 +966,45 @@ def test_required_ci_failure_waits_since_first_current_failure(self) -> None: self.assertEqual(waiting_since, current_facts["waiting_since"]) self.assertEqual(basis, current_facts["waiting_age_basis"]) + def test_override_command_clears_only_the_failures_that_predate_it(self) -> None: + facts = compute_facts( + { + "pr": { + "updatedAt": "2026-07-17T03:00:00Z", + "createdAt": "2026-07-14T01:00:00Z", + "author": {"login": "author"}, + "assignees": [], + "mergeStateStatus": "CLEAN", + "mergeable": "MERGEABLE", + }, + "checks": [ + {"bucket": "fail", "completed_at": "2026-07-17T01:00:00Z"}, + {"bucket": "fail", "completed_at": "2026-07-17T02:00:00Z"}, + {"bucket": "fail", "completed_at": "2026-07-17T05:00:00Z"}, + ], + "issue_comments": [ + { + "id": 7, + "user": {"login": "author"}, + "created_at": "2026-07-17T02:00:00Z", + "body": "/dashboard route:reviewers", + } + ], + }, + "author", + [], + ) + + self.assertEqual(3, facts["ci_failing_count"]) + self.assertEqual(1, facts["ci_uncleared_failing_count"]) + self.assertEqual("2026-07-17T05:00:00+00:00", facts["ci_uncleared_failing_since"]) + self.assertEqual("author", route_pr(facts, {}, 1)) + + add_wait_age_facts(facts, "author", {}) + + self.assertEqual("2026-07-17T05:00:00+00:00", facts["waiting_since"]) + self.assertEqual("ci_failure", facts["waiting_age_basis"]) + class LastActivityTest(unittest.TestCase): def _compute_last_activity_at(self, events: list[dict[str, object]]) -> object: diff --git a/.github/scripts/pull-request-dashboard/test_dashboard_override.py b/.github/scripts/pull-request-dashboard/test_dashboard_override.py index e47534beb0f..f75e93779f3 100644 --- a/.github/scripts/pull-request-dashboard/test_dashboard_override.py +++ b/.github/scripts/pull-request-dashboard/test_dashboard_override.py @@ -528,16 +528,12 @@ def test_unacknowledged_command_needs_current_authorization(self) -> None: self.assertEqual(1, facts["dashboard_override_cleared_count"]) def test_clears_check_failures_that_predate_the_command(self) -> None: - for ci_failing_latest, expected in ( - ("2026-07-30T11:00:00Z", True), - ("2026-07-30T13:00:00Z", False), - ): - with self.subTest(ci_failing_latest=ci_failing_latest): + for uncleared, expected in ((0, True), (1, False)): + with self.subTest(uncleared=uncleared): facts = { "dashboard_override_since": "2026-07-30T12:00:00Z", "ci_failing_count": 1, - "ci_failing_since": ci_failing_latest, - "ci_failing_latest": ci_failing_latest, + "ci_uncleared_failing_count": uncleared, } dashboard_override.clear_overridden_actions(facts, {}) @@ -548,14 +544,13 @@ def test_keeps_a_check_that_fails_after_the_command(self) -> None: facts = { "dashboard_override_since": "2026-07-30T12:00:00Z", "ci_failing_count": 2, - "ci_failing_since": "2026-07-30T11:00:00Z", - "ci_failing_latest": "2026-07-30T13:00:00Z", + "ci_uncleared_failing_count": 1, } dashboard_override.clear_overridden_actions(facts, {}) - self.assertFalse(facts["dashboard_override_cleared_ci"]) - self.assertEqual(2, dashboard_override.uncleared_ci_failing_count(facts)) + self.assertTrue(facts["dashboard_override_cleared_ci"]) + self.assertEqual(1, dashboard_override.uncleared_ci_failing_count(facts)) def test_clears_items_that_share_the_command_timestamp(self) -> None: # The command's own `...Z` timestamp is compared against `format_ts` @@ -563,7 +558,6 @@ def test_clears_items_that_share_the_command_timestamp(self) -> None: facts = { "dashboard_override_since": "2026-07-30T12:00:00Z", "ci_failing_count": 1, - "ci_failing_latest": "2026-07-30T12:00:00+00:00", } pending_actions = { "same": {"action": "author", "since": "2026-07-30T12:00:00+00:00"}, @@ -574,7 +568,6 @@ def test_clears_items_that_share_the_command_timestamp(self) -> None: self.assertEqual(["later"], sorted(remaining)) self.assertEqual(1, facts["dashboard_override_cleared_count"]) - self.assertTrue(facts["dashboard_override_cleared_ci"]) def test_clears_nothing_without_a_command(self) -> None: facts = {"dashboard_override_since": ""} 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 76670c4a5e7..ef4a5ef578b 100644 --- a/.github/scripts/pull-request-dashboard/test_pr_status_comment.py +++ b/.github/scripts/pull-request-dashboard/test_pr_status_comment.py @@ -180,7 +180,7 @@ def test_accuracy_note_bounds_report_url_for_large_status(self) -> None: { "route": "author", "facts": { - "ci_failing_count": 1, + "ci_uncleared_failing_count": 1, "non_blocking_check_failures": [ "&" * pr_status_comment.NON_BLOCKING_CHECK_FAILURE_NAME_LIMIT for _ in range( @@ -217,7 +217,7 @@ def test_waiting_on_author_names_required_ci_failure(self) -> None: self.pr(), { "route": "author", - "facts": {"author": "alice", "ci_failing_count": 1}, + "facts": {"author": "alice", "ci_uncleared_failing_count": 1}, }, ) @@ -233,7 +233,7 @@ def test_waiting_on_author_combines_ci_and_review_feedback_reasons(self) -> None "route": "author", "facts": { "author": "alice", - "ci_failing_count": 2, + "ci_uncleared_failing_count": 2, "author_action_review_thread_urls": [ "https://github.com/open-telemetry/example/pull/1#discussion_r1", ], @@ -252,7 +252,7 @@ def test_required_ci_action_notes_configured_non_blocking_failures(self) -> None { "route": "author", "facts": { - "ci_failing_count": 2, + "ci_uncleared_failing_count": 2, "non_blocking_check_failures": [ "CodeQL", "workflow-notification", @@ -273,7 +273,7 @@ def test_required_ci_action_escapes_non_blocking_failure_names(self) -> None: { "route": "author", "facts": { - "ci_failing_count": 1, + "ci_uncleared_failing_count": 1, "non_blocking_check_failures": [ "[CodeQL]