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
8 changes: 6 additions & 2 deletions .github/scripts/pull-request-dashboard/RATIONALE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions .github/scripts/pull-request-dashboard/notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down Expand Up @@ -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 "",
Expand Down
29 changes: 26 additions & 3 deletions .github/scripts/pull-request-dashboard/notify_slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")}
Expand All @@ -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 {}) != (
Expand All @@ -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:
Expand All @@ -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)],
Expand All @@ -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())))
Expand Down
17 changes: 12 additions & 5 deletions .github/workflows/pull-request-dashboard-repo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down