Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/scripts/pull-request-dashboard/CONTEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ diagnostics keep typed classification results and freeze only the source
discussion records.

`state.py` owns the JSON boundary. Its dashboard facts, stored-result, and state
codecs translate the immutable contracts to the version 13
codecs translate the immutable contracts to the version 15
`dashboard-state.json` shape. Malformed pull request entries are discarded
individually, so one bad entry does not prevent valid entries from loading.

Expand Down
16 changes: 11 additions & 5 deletions .github/scripts/pull-request-dashboard/RATIONALE.md
Original file line number Diff line number Diff line change
Expand Up @@ -336,11 +336,17 @@ the implementation understandable and operationally cheap.
with reviewers and is released to maintainers keeps its wait, because it never
left the people who owe it a response, and restarting there would present an
approval a week old as a merge request that just arrived.
- Maintenance-bot PRs retain maintainer-oriented routing because the bot cannot
respond to a dashboard action. Pending required checks affect the CI column
but never route one of these PRs to its author: a bot PR whose handoff is
held waits on reviewers instead. Merge conflicts remain visible without
overriding that routing.
- PRs from unattended automation never route to their author because the
automation cannot respond to a dashboard action. Copilot-delegated PRs are
different. The dashboard recovers a human assignee or the first commit's
human committer as the effective author, so that person can receive the
author route. Pending required checks affect the CI column, and a bot PR
whose handoff is held waits on reviewers instead. Known maintenance bots use
a one-approval threshold; other automation uses the repository's configured
threshold. Compatible cached state derives a missing author-capability fact
from the effective author identity and discards a stale author-routed result
for reevaluation, while an explicit stored value wins. Merge conflicts remain
visible without overriding that routing.
- A hold has a time limit, and past it the PR routes anyway. Every gate waits on
something outside the dashboard, and each one has been seen never to arrive: a
required check with no check run on the head, a Copilot review GitHub never
Expand Down
2 changes: 2 additions & 0 deletions .github/scripts/pull-request-dashboard/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@
assignees list[str] PR assignees.
is_maintenance_bot bool PR is authored by a
maintenance bot.
author_can_act bool Effective author can respond
to author-routed work.
is_draft bool
approval_count int Current unique APPROVED reviews
from approver-team members,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ class DashboardFacts:
copilot_review_stale: bool = False
copilot_review_needed: bool = False
is_maintenance_bot: bool = False
author_can_act: bool = True
is_draft: bool = False
approval_count: int = 0
conflicts: str = "unknown"
Expand Down
62 changes: 47 additions & 15 deletions .github/scripts/pull-request-dashboard/pull_request_evaluation.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
)
from github_cli import TransientGhError
from pull_request_source import (
Actor,
IssueComment,
PullRequestSource,
fetch_pull_request_source,
Expand All @@ -76,16 +77,46 @@
routing_failure_facts,
)
from routing_snapshot import build_routing_snapshot
from utils import format_ts, parse_ts
from utils import (
format_ts,
is_unattended_author_login,
normalize_author_identity,
parse_ts,
)


# Copilot appears under two slugs: `gh pr view`'s `author` field reports
# `app/copilot-swe-agent`, while the Pulls/commits endpoint's `committer.login`
# field can report the bare `copilot` slug. Either slug can name Copilot as the
# author, so the author set carries both while the committer set carries only
# the bare slug. These sets hold the identities `normalize_author_identity`
# returns, without the `app/` prefix or the `[bot]` suffix. Do not treat either
# slug as the human author behind a Copilot-authored PR.
_COPILOT_COMMITTER_IDENTITIES = {"copilot"}
_COPILOT_PR_AUTHOR_IDENTITIES = {"copilot-swe-agent", "copilot"}
_MAINTENANCE_APP_IDENTITIES = {"dependabot", "otelbot", "renovate"}


# Copilot appears in two API shapes: `gh pr view`'s `author` field uses the
# `app/<slug>` form, while the Pulls/commits endpoint's `committer.login`
# field can return the bare `copilot` slug. Do not treat either form as the
# human author behind a Copilot-authored PR.
_COPILOT_COMMITTER_LOGINS = {"copilot"}
_COPILOT_PR_AUTHORS = {"app/copilot-swe-agent", "copilot"}
_MAINTENANCE_BOT_PR_AUTHORS = {"app/otelbot", "app/renovate"}
def _is_maintenance_bot_author(login: str) -> bool:
normalized_login = (login or "").strip().casefold()
identity = normalize_author_identity(normalized_login)
return identity == "opentelemetrybot" or (
identity in _MAINTENANCE_APP_IDENTITIES
and (
normalized_login.startswith("app/")
or normalized_login.endswith("[bot]")
)
)


def _author_can_act(api_author: Actor, effective_author: str) -> bool:
if is_unattended_author_login(effective_author):
return False
return (
not api_author.is_bot
or normalize_author_identity(api_author.login)
!= normalize_author_identity(effective_author)
)


@dataclass(frozen=True)
Expand Down Expand Up @@ -116,12 +147,11 @@ def _human_author_for_copilot_pr(source: PullRequestSource) -> str:
for assignee in source.pull_request.assignees
]
for login in assignees:
low = login.lower()
identity = normalize_author_identity(login)
if (
login
and low not in _COPILOT_PR_AUTHORS
and not low.startswith("app/")
and not low.endswith("[bot]")
and identity not in _COPILOT_PR_AUTHOR_IDENTITIES
and not is_unattended_author_login(login)
):
return login

Expand All @@ -132,7 +162,8 @@ def _human_author_for_copilot_pr(source: PullRequestSource) -> str:
login = committer.login
if (
not login
or login.lower() in _COPILOT_COMMITTER_LOGINS
or normalize_author_identity(login) in _COPILOT_COMMITTER_IDENTITIES
or is_unattended_author_login(login)
or committer.is_bot
or committer.is_copilot_reviewer
):
Expand All @@ -142,7 +173,7 @@ def _human_author_for_copilot_pr(source: PullRequestSource) -> str:

def _effective_author(source: PullRequestSource) -> str:
author = source.pull_request.author.login
if author.lower() in _COPILOT_PR_AUTHORS:
if normalize_author_identity(author) in _COPILOT_PR_AUTHOR_IDENTITIES:
human_author = _human_author_for_copilot_pr(source)
if human_author:
return human_author
Expand Down Expand Up @@ -230,7 +261,8 @@ def _compute_facts(
copilot_review_exists=copilot_review_exists,
copilot_review_stale=copilot_review_stale,
copilot_review_needed=copilot_review_stale or copilot_review_findings,
is_maintenance_bot=api_author.lower() in _MAINTENANCE_BOT_PR_AUTHORS,
is_maintenance_bot=_is_maintenance_bot_author(api_author),
author_can_act=_author_can_act(pr.author, author),
is_draft=pr.is_draft,
approval_count=prepared_reviewers.approval_count,
conflicts=pr.conflicts,
Expand Down
8 changes: 4 additions & 4 deletions .github/scripts/pull-request-dashboard/routing_decision.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ def _base_route(
counts = _action_counts(pending_actions)
is_maintenance_bot = facts.is_maintenance_bot
approval_threshold = 1 if is_maintenance_bot else required_approvals
if (facts.ci_failing_count or 0) > 0 and not is_maintenance_bot:
if (facts.ci_failing_count or 0) > 0 and facts.author_can_act:
return DashboardRoute.AUTHOR
if counts["author"] and not is_maintenance_bot:
if counts["author"] and facts.author_can_act:
return DashboardRoute.AUTHOR
if facts.approval_count >= approval_threshold:
return DashboardRoute.MAINTAINER
Expand Down Expand Up @@ -145,12 +145,12 @@ def _hold_route_until_gates_settle(
) -> tuple[DashboardRoute, DashboardFacts]:
effective_previous_route = previous_route or DashboardRoute.AUTHOR
if effective_previous_route.value not in _ROUTE_PROGRESSION or (
facts.is_maintenance_bot
not facts.author_can_act
and effective_previous_route is DashboardRoute.AUTHOR
):
effective_previous_route = (
DashboardRoute.APPROVER
if facts.is_maintenance_bot
if not facts.author_can_act
else DashboardRoute.AUTHOR
)
gates_enabled = not bypass_gates
Expand Down
23 changes: 17 additions & 6 deletions .github/scripts/pull-request-dashboard/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
)
from github_cli import detect_repo, normalize_repo, repo_state_key
import state_branch
from utils import is_unattended_author_login


DASHBOARD_MARKDOWN_FILE = "pull-request-dashboard.md"
Expand All @@ -35,7 +36,7 @@
# current vector, ordinary state loaders may regenerate mismatched disposable
# caches. Every constant ending in _STATE_VERSION or _REVISION is included.
# dashboard-state.json: accepted PR routing results and backfill readiness.
DASHBOARD_STATE_VERSION = 13
DASHBOARD_STATE_VERSION = 15
# backfill-state.json: round-robin cursor used by full dashboard refreshes.
BACKFILL_STATE_VERSION = 3
# notification-state.json: pending and delivered Slack notification records.
Expand Down Expand Up @@ -453,8 +454,9 @@ def decode_dashboard_facts(value: Any) -> DashboardFacts:
raw_reviewers = []
if not isinstance(raw_reviewers, list):
raise ValueError("facts.reviewers must be an array")
author = _string(value.get("author", _MISSING), "facts.author")
return DashboardFacts(
author=_string(value.get("author", _MISSING), "facts.author"),
author=author,
assignees=_string_tuple(
value.get("assignees", _MISSING),
"facts.assignees",
Expand Down Expand Up @@ -516,6 +518,11 @@ def decode_dashboard_facts(value: Any) -> DashboardFacts:
value.get("is_maintenance_bot", _MISSING),
"facts.is_maintenance_bot",
),
author_can_act=_boolean(
value.get("author_can_act", _MISSING),
"facts.author_can_act",
not is_unattended_author_login(author),
),
is_draft=_boolean(
value.get("is_draft", _MISSING),
"facts.is_draft",
Expand Down Expand Up @@ -636,6 +643,7 @@ def encode_dashboard_facts(facts: DashboardFacts) -> dict[str, Any]:
"copilot_review_stale": facts.copilot_review_stale,
"copilot_review_needed": facts.copilot_review_needed,
"is_maintenance_bot": facts.is_maintenance_bot,
"author_can_act": facts.author_can_act,
Comment thread
trask marked this conversation as resolved.
"is_draft": facts.is_draft,
"approval_count": facts.approval_count,
"conflicts": facts.conflicts,
Expand Down Expand Up @@ -719,16 +727,19 @@ def decode_stored_result(
history = {}
if not isinstance(history, dict):
raise ValueError("dashboard result top_level_history must be an object")
facts = decode_dashboard_facts(
value["facts"] if "facts" in value else {}
)
if route is DashboardRoute.AUTHOR and not facts.author_can_act:
raise ValueError("dashboard result author route requires an actionable author")
return StoredDashboardResult(
pr_number=pr_number,
pr_url=_string(
value.get("pr_url", _MISSING),
"dashboard result pr_url",
),
route=route,
facts=decode_dashboard_facts(
value["facts"] if "facts" in value else {}
),
facts=facts,
top_level_history=freeze_json_object(history),
)

Expand Down Expand Up @@ -808,7 +819,7 @@ def load_dashboard_state_cache() -> DashboardState | None:
state = load_state_file(
dashboard_state_path(),
DASHBOARD_STATE_VERSION,
compatible_versions=(11, 12),
compatible_versions=(11, 12, 13),
)
if state is None:
return None
Expand Down
Loading