From e0120fc09f8df914ac89c1d6cd7d8c6b7a03ba1a Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Fri, 26 Jun 2026 09:55:40 -0700 Subject: [PATCH] Scope PR dashboard Slack notifications --- .../pull-request-dashboard/RATIONALE.md | 8 +++-- .../pull-request-dashboard/notifications.py | 3 ++ .../pull-request-dashboard/notify_slack.py | 29 +++++++++++++++++-- .../workflows/pull-request-dashboard-repo.yml | 17 +++++++---- 4 files changed, 47 insertions(+), 10 deletions(-) diff --git a/.github/scripts/pull-request-dashboard/RATIONALE.md b/.github/scripts/pull-request-dashboard/RATIONALE.md index 38925548..70986fcb 100644 --- a/.github/scripts/pull-request-dashboard/RATIONALE.md +++ b/.github/scripts/pull-request-dashboard/RATIONALE.md @@ -66,11 +66,15 @@ the implementation understandable and operationally cheap. - Slack notification state is PR-granular. It does not track notification history separately for each assignee. - When notification state is first created, existing approver-routed PRs may - receive initial notifications on the next run. Avoiding that bootstrap case - would require storing separate seen-but-not-notified state. + receive initial notifications on a later targeted refresh. Avoiding that + bootstrap case would require storing separate seen-but-not-notified state. - When a mapped assignee is added after a PR was already notified during the same waiting period, that assignee may wait until the next follow-up cadence instead of receiving an immediate initial notification. +- Scheduled runs send only due follow-up reminders. Targeted PR refreshes send + only the triggering PR's initial notification. This keeps webhook-driven + refreshes from sweeping unrelated PR reminders while preserving the hourly + reminder pass. - Slack notifications are sent only for dashboard state that has already been accepted on the state branch. A newer dashboard update can land after the notification job checks out state, so a notification can be slightly late diff --git a/.github/scripts/pull-request-dashboard/notifications.py b/.github/scripts/pull-request-dashboard/notifications.py index fd61cee2..fd345981 100644 --- a/.github/scripts/pull-request-dashboard/notifications.py +++ b/.github/scripts/pull-request-dashboard/notifications.py @@ -191,6 +191,7 @@ def next_notification_state( previous_state: dict[str, Any], now: datetime, notification_numbers: set[int] | None = None, + notification_kinds: set[str] | None = None, ) -> dict[str, Any]: previous_prs = previous_state.get("prs") or {} previous_state_exists = bool(previous_state.get("_loaded_from_dashboard")) @@ -236,6 +237,8 @@ def next_notification_state( kind = pending_notification_kind( previous_state_exists, previous_pr_state, current_waiting_since, now, ) + if kind and notification_kinds is not None and kind not in notification_kinds: + kind = None new_pr_state: dict[str, Any] = { "last_notified_at": previous_pr_state.get("last_notified_at") or "", diff --git a/.github/scripts/pull-request-dashboard/notify_slack.py b/.github/scripts/pull-request-dashboard/notify_slack.py index d48ae3d4..ce12a284 100644 --- a/.github/scripts/pull-request-dashboard/notify_slack.py +++ b/.github/scripts/pull-request-dashboard/notify_slack.py @@ -27,6 +27,8 @@ def notify_slack_from_state( repo: str, prior_notification_state: Path | None, + notification_numbers: set[int] | None, + notification_kinds: set[str] | None, ) -> list[str]: prs = list_open_prs(repo) open_pr_numbers = {p["number"] for p in prs if not p.get("isDraft")} @@ -47,6 +49,8 @@ def notify_slack_from_state( results, previous_state, utc_now(), + notification_numbers=notification_numbers, + notification_kinds=notification_kinds, ) notification_errors = [str(error) for error in notification_state.get("_notification_errors") or []] notification_state_changed = (notification_state.get("prs") or {}) != ( @@ -68,8 +72,14 @@ def notification_errors_path() -> Path: return Path(os.environ.get("RUNNER_TEMP", ".")) / "notification-errors.txt" -def notify_slack(repo: str, prior_notification_state: Path, notification_errors: Path) -> int: - errors = notify_slack_from_state(repo, prior_notification_state) +def notify_slack( + repo: str, + prior_notification_state: Path, + notification_errors: Path, + notification_numbers: set[int] | None, + notification_kinds: set[str] | None, +) -> int: + errors = notify_slack_from_state(repo, prior_notification_state, notification_numbers, notification_kinds) if errors: notification_errors.write_text("\n".join(errors) + "\n", encoding="utf-8") else: @@ -82,10 +92,18 @@ def notify_slack_with_state(args: argparse.Namespace, state_dir: Path) -> int: prior_notification_state = prior_notification_state_path() notification_errors = notification_errors_path() notification_errors.unlink(missing_ok=True) + notification_numbers = {args.pr_number} if args.pr_number is not None else None + notification_kinds = {"initial"} if args.pr_number is not None else {"follow-up"} status = state_branch.push_state_changes( state_dir, "Update dashboard notification state", - lambda: notify_slack(normalize_repo(args.repo) if args.repo else detect_repo(), prior_notification_state, notification_errors), + lambda: notify_slack( + normalize_repo(args.repo) if args.repo else detect_repo(), + prior_notification_state, + notification_errors, + notification_numbers, + notification_kinds, + ), state_branch=args.state_branch, add_paths=[f"{repo_key}/notification-state.json"], retry_snapshots=[(notification_state_path(), prior_notification_state)], @@ -107,6 +125,11 @@ def main() -> int: required=True, help="git branch used for workflow state", ) + parser.add_argument( + "--pr-number", + type=int, + help="send initial notifications for one pull request; omit to send due follow-up notifications only", + ) args = parser.parse_args() with state_branch.temporary_state_dir() as state_dir: set_state_dir(state_dir / (repo_state_key(args.repo) if args.repo else repo_state_key(detect_repo()))) diff --git a/.github/workflows/pull-request-dashboard-repo.yml b/.github/workflows/pull-request-dashboard-repo.yml index 9b65d021..e34bbfbb 100644 --- a/.github/workflows/pull-request-dashboard-repo.yml +++ b/.github/workflows/pull-request-dashboard-repo.yml @@ -209,9 +209,11 @@ jobs: notify-slack: needs: update-dashboard - if: needs.update-dashboard.result == 'success' + if: >- + needs.update-dashboard.result == 'success' && + (inputs.trigger_event == 'schedule' || inputs.pr_number != '') concurrency: - group: ${{ github.workflow }}-notify-${{ inputs.repository }} + group: ${{ github.workflow }}-notify-${{ inputs.repository }}-${{ inputs.pr_number || 'full' }} cancel-in-progress: false permissions: contents: write @@ -240,10 +242,15 @@ jobs: SLACK_CHANNEL: ${{ inputs.slack_channel }} SLACK_USER_MAP_JSON: ${{ inputs.slack_user_mapping_json }} REPO_NAME: ${{ inputs.repository }} + TRIGGER_PR_NUMBER: ${{ inputs.pr_number }} run: | - python3 .github/scripts/pull-request-dashboard/notify_slack.py \ - --state-branch "$DASHBOARD_STATE_BRANCH" \ - --repo "$REPO_NAME" + set -euo pipefail + notify_args=(--state-branch "$DASHBOARD_STATE_BRANCH" --repo "$REPO_NAME") + if [[ -n "${TRIGGER_PR_NUMBER:-}" ]]; then + notify_args+=(--pr-number "$TRIGGER_PR_NUMBER") + fi + + python3 .github/scripts/pull-request-dashboard/notify_slack.py "${notify_args[@]}" publish-dashboard: needs: update-dashboard