diff --git a/.github/scripts/pull-request-dashboard/RATIONALE.md b/.github/scripts/pull-request-dashboard/RATIONALE.md index 486e1027a53..bb7cb7fe07e 100644 --- a/.github/scripts/pull-request-dashboard/RATIONALE.md +++ b/.github/scripts/pull-request-dashboard/RATIONALE.md @@ -26,6 +26,13 @@ the implementation understandable and operationally cheap. repository's current open PR list. - The dashboard issue is discovered dynamically by title and label, so target repositories do not need to store issue numbers in config. +- Refresh events that carry no pull request number report the head commit + instead, and the workflow resolves it to an open pull request. GitHub omits + the pull request association from check and status events whose head branch + lives in a fork, which is where nearly every contribution comes from, so + without this the CI columns of those PRs would only refresh on the hourly + backfill. The webhook bridge cannot resolve the commit itself, because its + GitHub App is installed only on `shared-workflows`. ## Workflow Concurrency @@ -180,6 +187,16 @@ the implementation understandable and operationally cheap. holds the merge, so it is reported as failing rather than skipped. Tools with no such check are not reported, because GitHub expects results only from the tool configurations that actually ran. +- That `NEUTRAL` is only reported as failing once every check at the head has + finished, including optional ones. The code scanning app publishes the tool + check as `NEUTRAL` before the analysis is uploaded and then replaces it in + place, so an analysis still running is reported as pending instead of pinning + the PR to its author. The replacement leaves the enclosing check suite + untouched, so the dashboard subscribes to code scanning check runs to observe + the final result. Check runs from other apps are ignored, because their check + suite reports them and one refresh per job would multiply webhook volume. +- Required checks reported as commit statuses, such as EasyCLA, never belong to + a check suite, so commit status events are subscribed as well. - A failing required status check routes a human-authored PR to the author before discussion and approval routing. The live PR status comment names the CI failure, including when review feedback also needs author action. diff --git a/.github/scripts/pull-request-dashboard/WEBHOOK_SETUP.md b/.github/scripts/pull-request-dashboard/WEBHOOK_SETUP.md index edb7538f66f..1bbd7e057c6 100644 --- a/.github/scripts/pull-request-dashboard/WEBHOOK_SETUP.md +++ b/.github/scripts/pull-request-dashboard/WEBHOOK_SETUP.md @@ -48,6 +48,7 @@ openssl rand -hex 32 Repository permissions: - Checks: read-only +- Commit statuses: read-only - Contents: read-only - Issues: read and write - Metadata: read-only @@ -61,7 +62,8 @@ Permission rationale: | Permission | Access | Why it is needed | | ---------- | ------ | ---------------- | -| Checks | Read | Required to subscribe to check-suite events and to read check data for dashboard rows. | +| Checks | Read | Required to subscribe to check-suite and check-run events and to read check data for dashboard rows. | +| Commit statuses | Read | Required to subscribe to commit status events, which are the only notification for checks reported as statuses instead of check runs, and to read those status contexts in the check rollup. | | Contents | Read | Reads PR commits and repository metadata needed by pull/commit APIs. | | Issues | Read and write | Finds, creates, and updates the dashboard issue. | | Metadata | Read | Required by GitHub for GitHub App repository access. | @@ -76,23 +78,27 @@ a pull request, GitHub governs writing it with the `Pull requests` permission; Subscribe to events: +- Check run - Check suite - Pull request - Issue comment - Pull request review - Pull request review comment - Pull request review thread +- Status Event rationale: | Event | Why it is needed | | ----- | ---------------- | +| Check run | Refreshes CI status when the code scanning app replaces its placeholder result in place, which does not update the enclosing check suite. Check runs reported by other apps are ignored because their check suite already reports them. | | Check suite | Refreshes CI status when checks complete. | | Pull request | Refreshes dashboard rows when PR state, draft status, labels, assignees, branches, or metadata change. | | Issue comment | Refreshes PR conversation state when PR issue comments are created, edited, or deleted. Events generated by the dashboard App changing its own comments are ignored. | | Pull request review | Refreshes approval/change-request state and the live PR status comment. | | Pull request review comment | Refreshes inline review-comment discussion state. | | Pull request review thread | Refreshes when inline review threads are resolved or unresolved. | +| Status | Refreshes CI status for required checks reported as commit statuses, such as EasyCLA, which are never part of a check suite. Statuses on the default branch are ignored. | Create the app, update the logo, and generate a private key. @@ -156,6 +162,7 @@ The webhook bridge should dispatch `pull-request-dashboard.yml` in { "repository": "opentelemetry-java-instrumentation", "pr_number": "12345", + "head_sha": "", "trigger_event": "pull_request_review_comment" } ``` @@ -165,6 +172,12 @@ Notes: - `repository` is the short repository name under `open-telemetry`, and must match a `repositories.json` entry exactly. An owner-prefixed name is rejected. - Omit `pr_number` or set it to an empty string for a backfill. -- The central workflow validates `repository` and `pr_number` before using them. - `trigger_event` only selects a concurrency group, so it is validated on the - backfill path only; the bridge is what restricts it to known event names. +- Send `head_sha` instead of `pr_number` when the event carries no pull request + number. Check and status events for a pull request whose head branch lives in + a fork report no pull request association, so the central workflow resolves + the head commit to an open pull request and skips the refresh when there is + none. +- The central workflow validates `repository`, `pr_number` and `head_sha` before + using them. `trigger_event` only selects a concurrency group, so it is + validated on the backfill path only; the bridge is what restricts it to known + event names. diff --git a/.github/scripts/pull-request-dashboard/github_cli.py b/.github/scripts/pull-request-dashboard/github_cli.py index 2d6a5224828..ba304b375bb 100644 --- a/.github/scripts/pull-request-dashboard/github_cli.py +++ b/.github/scripts/pull-request-dashboard/github_cli.py @@ -334,6 +334,7 @@ def fetch_pr_reviews(owner: str, repo_name: str, number: int) -> list[dict[str, databaseId } workflowRun { + databaseId workflow { name } @@ -384,6 +385,7 @@ def normalize_check(node: dict[str, Any]) -> dict[str, Any]: "state": state, "bucket": check_bucket(state), "workflow": workflow.get("name") or "", + "workflow_run_id": workflow_run.get("databaseId"), "description": node.get("description") or "", "link": (node.get("targetUrl") if is_status else node.get("detailsUrl")) or "", "started_at": (node.get("createdAt") if is_status else node.get("startedAt")) or "", @@ -404,6 +406,18 @@ def check_attempt_order(check: dict[str, Any]) -> tuple[int, int | str]: return 1, check["check_run_id"] or 0 +def check_identity(check: dict[str, Any]) -> tuple[str, str, int | None]: + # Separate workflows can name a job identically, so the workflow keeps their + # checks apart while rerun attempts of one check still collapse. + return check["workflow"], check["name"], check["integration_id"] + + +def check_execution_identity(check: dict[str, Any]) -> tuple[int | None, str, int | None]: + # Two runs of one workflow can be in flight for the same commit, so a check + # that finished in one run must not hide the one still running in another. + return check["workflow_run_id"], check["name"], check["integration_id"] + + def gh_pr_check_rollup( repo: str, pr_id: str, @@ -411,9 +425,10 @@ def gh_pr_check_rollup( ) -> dict[str, list[dict[str, Any]]] | None: del repo checks_by_identity: dict[ - tuple[str, int | None, bool], tuple[dict[str, Any], bool] + tuple[str, str, int | None, bool], tuple[dict[str, Any], bool] ] = {} - code_scanning_by_identity: dict[tuple[str, int | None], dict[str, Any]] = {} + latest_by_execution: dict[tuple[int | None, str, int | None], dict[str, Any]] = {} + code_scanning_executions: set[tuple[int | None, str, int | None]] = set() after: str | None = None try: while True: @@ -428,26 +443,25 @@ def gh_pr_check_rollup( is_required = bool(node.get("isRequired")) name = (node.get("context") or node.get("name") or "") app = ((node.get("checkSuite") or {}).get("app") or {}) + check = normalize_check(node) + identity = check_identity(check) + execution = check_execution_identity(check) + latest_attempt = latest_by_execution.get(execution) + if latest_attempt is None or check_attempt_order( + check + ) >= check_attempt_order(latest_attempt): + latest_by_execution[execution] = check if app.get("databaseId") == CODE_SCANNING_APP_ID: - check = normalize_check(node) - code_scanning_identity = (check["name"], check["integration_id"]) - previous_attempt = code_scanning_by_identity.get( - code_scanning_identity - ) - if previous_attempt is None or check_attempt_order( - check - ) >= check_attempt_order(previous_attempt): - code_scanning_by_identity[code_scanning_identity] = check + code_scanning_executions.add(execution) if not is_required and not any( fnmatchcase(name, pattern) for pattern in non_blocking_check_patterns ): continue - check = normalize_check(node) - identity = (check["name"], check["integration_id"], is_required) - previous = checks_by_identity.get(identity) + required_identity = (*identity, is_required) + previous = checks_by_identity.get(required_identity) if previous is None or check_attempt_order(check) >= check_attempt_order(previous[0]): - checks_by_identity[identity] = (check, is_required) + checks_by_identity[required_identity] = (check, is_required) page_info = contexts.get("pageInfo") or {} if not page_info.get("hasNextPage"): break @@ -464,7 +478,19 @@ def gh_pr_check_rollup( for check, is_required in checks if not is_required and check.get("bucket") in ("fail", "cancel") ], - "code_scanning": list(code_scanning_by_identity.values()), + "code_scanning": [ + check + for execution, check in latest_by_execution.items() + if execution in code_scanning_executions + ], + # Every check at the head, not just the required ones, because the + # optional job that uploads a code scanning analysis is what decides + # whether an undetermined result is final. + "pending": [ + check + for check in latest_by_execution.values() + if check["bucket"] == "pending" + ], } @@ -522,17 +548,24 @@ def code_scanning_tools(rules: list[dict[str, Any]] | None) -> list[str]: def required_code_scanning_checks( code_scanning_checks: list[dict[str, Any]], tools: list[str], + checks_still_running: bool, ) -> list[dict[str, Any]]: # A code_scanning ruleset rule holds the merge on a check named after the # tool, but GitHub never reports that check as required. NEUTRAL there means # the alerts introduced by the PR could not be determined, which also holds - # the merge, so it cannot be treated as a skip. + # the merge, so it cannot be treated as a skip. While other checks are still + # running that NEUTRAL is usually a placeholder for an analysis that has not + # been uploaded yet, and code scanning replaces it in place without any + # event that would refresh a stale failure. required: list[dict[str, Any]] = [] for check in code_scanning_checks: if check["name"] not in tools: continue if check["bucket"] == "skipping": - check = {**check, "bucket": "fail"} + check = { + **check, + "bucket": "pending" if checks_still_running else "fail", + } required.append(check) return required @@ -900,6 +933,7 @@ def fetch_pr_routing_raw( required_code_scanning_checks( check_rollup["code_scanning"], code_scanning_tools(branch_rules), + bool(check_rollup["pending"]), ), ) return { diff --git a/.github/scripts/pull-request-dashboard/netlify/functions/github-webhook.mjs b/.github/scripts/pull-request-dashboard/netlify/functions/github-webhook.mjs index 615e1883f86..6e87245ed2f 100644 --- a/.github/scripts/pull-request-dashboard/netlify/functions/github-webhook.mjs +++ b/.github/scripts/pull-request-dashboard/netlify/functions/github-webhook.mjs @@ -7,11 +7,15 @@ const WORKFLOW_REPOSITORY = "shared-workflows"; const WORKFLOW_ID = "pull-request-dashboard.yml"; const WORKFLOW_REF = "main"; const DASHBOARD_APP_SLUG = "opentelemetry-pr-dashboard"; +const CODE_SCANNING_APP_ID = 57789; // github-advanced-security const ALLOWED_ACTIONS = { // GitHub only delivers `requested` and `rerequested` to apps with write-level // Checks access; this app has read-only, so `completed` is all that arrives. check_suite: new Set(["completed"]), + check_run: new Set(["completed"]), + // Commit statuses carry no action. + status: new Set([""]), pull_request: new Set([ "assigned", "closed", @@ -67,13 +71,19 @@ async function handle(request) { } const payload = parseJson(rawBody); - const action = payload.action; + const action = payload.action || ""; if (!isAllowedAction(eventName, action)) { return response(202, { status: "ignored", reason: `unsupported action: ${eventName}.${action || "missing"}` }); } if (isDashboardSelfTriggeredCommentEvent(eventName, payload)) { return response(202, { status: "ignored", reason: "dashboard-managed comment" }); } + if (isRedundantCheckRunEvent(eventName, payload)) { + return response(202, { status: "ignored", reason: "check run covered by its check suite" }); + } + if (isDefaultBranchStatusEvent(eventName, payload)) { + return response(202, { status: "ignored", reason: "status on the default branch" }); + } const repository = readRepository(payload); if (!repository.fullName) { @@ -84,8 +94,11 @@ async function handle(request) { } const prNumber = extractPullRequestNumber(eventName, payload); - if (!Number.isInteger(prNumber) || prNumber <= 0) { - return response(202, { status: "ignored", reason: "no pull request number found" }); + const headSha = extractHeadSha(eventName, payload); + const dispatchPrNumber = Number.isInteger(prNumber) && prNumber > 0 ? String(prNumber) : ""; + const dispatchHeadSha = dispatchPrNumber ? "" : headSha; + if (!dispatchPrNumber && !dispatchHeadSha) { + return response(202, { status: "ignored", reason: "no pull request number or head commit found" }); } const dispatcherJwt = createAppJwt({ appId: config.dispatcherAppId, privateKey: config.dispatcherPrivateKey }); @@ -93,14 +106,16 @@ async function handle(request) { const installationToken = await createInstallationToken(dispatcherJwt, installationId); await dispatchWorkflow(installationToken, { repository: repository.name, - pr_number: String(prNumber), + pr_number: dispatchPrNumber, + head_sha: dispatchHeadSha, trigger_event: eventName, }); return response(202, { status: "dispatched", repository: repository.fullName, - pr_number: prNumber, + pr_number: dispatchPrNumber, + head_sha: dispatchHeadSha, trigger_event: eventName, }); } @@ -109,6 +124,28 @@ export function isAllowedAction(eventName, action) { return Boolean(ALLOWED_ACTIONS[eventName] && ALLOWED_ACTIONS[eventName].has(action)); } +// Every other app reports its check runs under a check suite that completes +// with them, and one dispatch per job would multiply webhook volume. The code +// scanning app instead rewrites its already completed check run when the +// analysis arrives, which nothing else reports. +export function isRedundantCheckRunEvent(eventName, payload) { + if (eventName !== "check_run") { + return false; + } + const app = (payload.check_run || {}).app || {}; + return app.id !== CODE_SCANNING_APP_ID; +} + +export function isDefaultBranchStatusEvent(eventName, payload) { + if (eventName !== "status") { + return false; + } + const defaultBranch = (payload.repository || {}).default_branch; + return (payload.branches || []).some( + (branch) => branch && branch.name === defaultBranch, + ); +} + export function isDashboardSelfTriggeredCommentEvent(eventName, payload) { if (eventName !== "issue_comment") { return false; @@ -174,7 +211,7 @@ function parseJson(rawBody) { } } -function extractPullRequestNumber(eventName, payload) { +export function extractPullRequestNumber(eventName, payload) { if (eventName === "issue_comment") { if (!payload.issue || !payload.issue.pull_request) { return undefined; @@ -182,8 +219,11 @@ function extractPullRequestNumber(eventName, payload) { return payload.issue.number; } - const checkPullRequestNumber = extractPullRequestNumberFromCheckSuitePullRequests( - payload.check_suite && payload.check_suite.pull_requests, + // Check suites and check runs on a fork head report an empty association, + // because GitHub only matches them to a pull request whose head branch lives + // in this repository. Those events fall back to the head SHA. + const checkPullRequestNumber = extractPullRequestNumberFromCheckPullRequests( + checkPullRequests(payload), payload.repository, ); if (checkPullRequestNumber) { @@ -201,7 +241,20 @@ function extractPullRequestNumber(eventName, payload) { ]); } -function extractPullRequestNumberFromCheckSuitePullRequests(pullRequests, repository) { +export function extractHeadSha(eventName, payload) { + const sha = + eventName === "status" + ? payload.sha + : (payload.check_suite || payload.check_run || {}).head_sha; + return typeof sha === "string" && /^[0-9a-f]{40}$/.test(sha) ? sha : ""; +} + +function checkPullRequests(payload) { + const source = payload.check_suite || payload.check_run || {}; + return source.pull_requests; +} + +function extractPullRequestNumberFromCheckPullRequests(pullRequests, repository) { if (!Array.isArray(pullRequests)) { return undefined; } @@ -209,7 +262,7 @@ function extractPullRequestNumberFromCheckSuitePullRequests(pullRequests, reposi if ( pullRequest && Number.isInteger(pullRequest.number) && - checkSuitePullRequestBelongsToRepository(pullRequest, repository) + checkPullRequestBelongsToRepository(pullRequest, repository) ) { return pullRequest.number; } @@ -217,7 +270,7 @@ function extractPullRequestNumberFromCheckSuitePullRequests(pullRequests, reposi return undefined; } -function checkSuitePullRequestBelongsToRepository(pullRequest, repository) { +function checkPullRequestBelongsToRepository(pullRequest, repository) { const repositoryUrl = repository && repository.url; if (!repositoryUrl) { return false; diff --git a/.github/scripts/pull-request-dashboard/test_author_nudge.py b/.github/scripts/pull-request-dashboard/test_author_nudge.py index eabb2ecad48..1169bd0a815 100644 --- a/.github/scripts/pull-request-dashboard/test_author_nudge.py +++ b/.github/scripts/pull-request-dashboard/test_author_nudge.py @@ -116,6 +116,7 @@ def test_routing_fingerprint_tracks_base_branch(self) -> None: "required": [{"name": "build", "bucket": "fail"}], "non_blocking_failures": [], "code_scanning": [], + "pending": [], }, ) @patch("github_cli.fetch_review_threads", return_value=[]) diff --git a/.github/scripts/pull-request-dashboard/test_dashboard.py b/.github/scripts/pull-request-dashboard/test_dashboard.py index 7a22edec160..ab8653ccc60 100644 --- a/.github/scripts/pull-request-dashboard/test_dashboard.py +++ b/.github/scripts/pull-request-dashboard/test_dashboard.py @@ -163,6 +163,7 @@ def gh_api(path: str, paginate: bool) -> list[dict]: "required": [], "non_blocking_failures": [], "code_scanning": [], + "pending": [], }, ), patch("github_cli.gh_branch_rules", return_value=[]), diff --git a/.github/scripts/pull-request-dashboard/test_github_cli.py b/.github/scripts/pull-request-dashboard/test_github_cli.py index 6003c4276c5..2160efc77ad 100644 --- a/.github/scripts/pull-request-dashboard/test_github_cli.py +++ b/.github/scripts/pull-request-dashboard/test_github_cli.py @@ -379,6 +379,7 @@ def test_gh_pr_checks_preserves_reporting_app_identity(self, graphql) -> None: { "__typename": "CheckRun", "name": "optional", + "url": "https://github.com/open-telemetry/example/runs/87974237001", "isRequired": False, }, ], @@ -628,6 +629,22 @@ def test_undetermined_code_scanning_result_is_a_failure(self) -> None: for check in required_code_scanning_checks( checks, ["CodeQL", "zizmor"], + False, + ) + ], + ) + + def test_undetermined_code_scanning_result_is_pending_while_checks_run( + self, + ) -> None: + self.assertEqual( + [("CodeQL", "pending")], + [ + (check["name"], check["bucket"]) + for check in required_code_scanning_checks( + [{"name": "CodeQL", "bucket": "skipping", "state": "NEUTRAL"}], + ["CodeQL"], + True, ) ], ) @@ -638,6 +655,7 @@ def test_code_scanning_checks_are_ignored_without_a_ruleset_rule(self) -> None: required_code_scanning_checks( [{"name": "CodeQL", "bucket": "skipping"}], [], + False, ), ) @@ -687,6 +705,226 @@ def test_check_rollup_keeps_the_latest_code_scanning_attempt(self, graphql) -> N [(check["name"], check["bucket"]) for check in rollup["code_scanning"]], ) + @patch("github_cli.gh_graphql") + def test_check_rollup_reports_unfinished_optional_checks_as_pending( + self, + graphql, + ) -> None: + graphql.return_value = _rollup_page([ + { + "__typename": "CheckRun", + "name": "CodeQL", + "status": "COMPLETED", + "conclusion": "NEUTRAL", + "url": "https://github.com/open-telemetry/example/runs/1", + "isRequired": False, + "checkSuite": {"app": {"databaseId": 57789}}, + }, + { + "__typename": "CheckRun", + "name": "Analyze (java)", + "status": "IN_PROGRESS", + "url": "https://github.com/open-telemetry/example/runs/2", + "isRequired": False, + "checkSuite": {"app": {"databaseId": 15368}}, + }, + ]) + + rollup = gh_pr_check_rollup("open-telemetry/example", "PR_id", []) + + self.assertIsNotNone(rollup) + self.assertEqual( + ["Analyze (java)"], + [check["name"] for check in rollup["pending"]], + ) + + @patch("github_cli.gh_graphql") + def test_check_rollup_keeps_same_named_checks_from_separate_workflows( + self, + graphql, + ) -> None: + graphql.return_value = _rollup_page([ + { + "__typename": "CheckRun", + "name": "test", + "status": "COMPLETED", + "conclusion": "FAILURE", + "url": "https://github.com/open-telemetry/example/runs/1", + "isRequired": True, + "checkSuite": { + "app": {"databaseId": 15368}, + "workflowRun": { + "databaseId": 100, + "workflow": {"name": "build"}, + }, + }, + }, + { + "__typename": "CheckRun", + "name": "test", + "status": "COMPLETED", + "conclusion": "SUCCESS", + "url": "https://github.com/open-telemetry/example/runs/2", + "isRequired": True, + "checkSuite": { + "app": {"databaseId": 15368}, + "workflowRun": { + "databaseId": 200, + "workflow": {"name": "native-tests"}, + }, + }, + }, + ]) + + rollup = gh_pr_check_rollup("open-telemetry/example", "PR_id", []) + + self.assertIsNotNone(rollup) + self.assertEqual( + [("build", "fail"), ("native-tests", "pass")], + sorted( + (check["workflow"], check["bucket"]) + for check in rollup["required"] + ), + ) + + @patch("github_cli.gh_graphql") + def test_check_rollup_keeps_same_named_checks_from_separate_runs( + self, + graphql, + ) -> None: + graphql.return_value = _rollup_page([ + { + "__typename": "CheckRun", + "name": "test", + "status": "IN_PROGRESS", + "url": "https://github.com/open-telemetry/example/runs/1", + "isRequired": False, + "checkSuite": { + "app": {"databaseId": 15368}, + "workflowRun": { + "databaseId": 100, + "workflow": {"name": "build"}, + }, + }, + }, + { + "__typename": "CheckRun", + "name": "test", + "status": "COMPLETED", + "conclusion": "SUCCESS", + "url": "https://github.com/open-telemetry/example/runs/2", + "isRequired": False, + "checkSuite": { + "app": {"databaseId": 15368}, + "workflowRun": { + "databaseId": 200, + "workflow": {"name": "build"}, + }, + }, + }, + ]) + + rollup = gh_pr_check_rollup("open-telemetry/example", "PR_id", []) + + self.assertIsNotNone(rollup) + self.assertEqual( + [(100, "test")], + [ + (check["workflow_run_id"], check["name"]) + for check in rollup["pending"] + ], + ) + + @patch("github_cli.gh_graphql") + def test_check_rollup_collapses_rerun_attempts_of_one_run( + self, + graphql, + ) -> None: + graphql.return_value = _rollup_page([ + { + "__typename": "CheckRun", + "name": "test", + "status": "IN_PROGRESS", + "url": "https://github.com/open-telemetry/example/runs/1", + "isRequired": False, + "checkSuite": { + "app": {"databaseId": 15368}, + "workflowRun": { + "databaseId": 100, + "workflow": {"name": "build"}, + }, + }, + }, + { + "__typename": "CheckRun", + "name": "test", + "status": "COMPLETED", + "conclusion": "SUCCESS", + "url": "https://github.com/open-telemetry/example/runs/2", + "isRequired": False, + "checkSuite": { + "app": {"databaseId": 15368}, + "workflowRun": { + "databaseId": 100, + "workflow": {"name": "build"}, + }, + }, + }, + ]) + + rollup = gh_pr_check_rollup("open-telemetry/example", "PR_id", []) + + self.assertIsNotNone(rollup) + self.assertEqual([], rollup["pending"]) + + @patch("github_cli.gh_graphql") + def test_required_check_keeps_only_the_latest_attempt_of_a_name( + self, + graphql, + ) -> None: + # A superseded run leaves a cancelled check behind that the run which + # replaced it has already passed. + graphql.return_value = _rollup_page([ + { + "__typename": "CheckRun", + "name": "changelog", + "status": "COMPLETED", + "conclusion": "CANCELLED", + "url": "https://github.com/open-telemetry/example/runs/1", + "isRequired": True, + "checkSuite": { + "app": {"databaseId": 15368}, + "workflowRun": { + "databaseId": 100, + "workflow": {"name": "Changelog"}, + }, + }, + }, + { + "__typename": "CheckRun", + "name": "changelog", + "status": "COMPLETED", + "conclusion": "SUCCESS", + "url": "https://github.com/open-telemetry/example/runs/2", + "isRequired": True, + "checkSuite": { + "app": {"databaseId": 15368}, + "workflowRun": { + "databaseId": 200, + "workflow": {"name": "Changelog"}, + }, + }, + }, + ]) + + rollup = gh_pr_check_rollup("open-telemetry/example", "PR_id", []) + + self.assertIsNotNone(rollup) + self.assertEqual( + [("changelog", "pass")], + [(check["name"], check["bucket"]) for check in rollup["required"]], + ) + @patch("github_cli.gh_graphql") def test_required_code_scanning_context_keeps_normal_classification( self, @@ -781,6 +1019,7 @@ def test_check_rollup_separates_code_scanning_results(self, graphql) -> None: "code_scanning": [ {"name": "CodeQL", "bucket": "skipping", "integration_id": 57789}, ], + "pending": [], }, ) @patch("github_cli.fetch_review_threads", return_value=[]) diff --git a/.github/scripts/pull-request-dashboard/test_github_webhook.mjs b/.github/scripts/pull-request-dashboard/test_github_webhook.mjs index 052492d6c84..648afd70a16 100644 --- a/.github/scripts/pull-request-dashboard/test_github_webhook.mjs +++ b/.github/scripts/pull-request-dashboard/test_github_webhook.mjs @@ -2,12 +2,18 @@ import assert from "node:assert/strict"; import test from "node:test"; import { + extractHeadSha, + extractPullRequestNumber, isAllowedAction, isDashboardSelfTriggeredCommentEvent, + isDefaultBranchStatusEvent, + isRedundantCheckRunEvent, } from "./netlify/functions/github-webhook.mjs"; const dashboardApp = { slug: "opentelemetry-pr-dashboard" }; const dashboardActor = { id: 1 }; +const codeScanningApp = { id: 57789 }; +const headSha = "65325a64c9b2e4b8a1d0f3c7e5a9b1d2c3e4f5a6"; test("refreshes when the dashboard override label changes", () => { assert.equal(isAllowedAction("pull_request", "labeled"), true); @@ -25,6 +31,71 @@ test("refreshes only once a check suite completes", () => { assert.equal(isAllowedAction("check_suite", "rerequested"), false); }); +test("refreshes when a check run completes", () => { + assert.equal(isAllowedAction("check_run", "completed"), true); + assert.equal(isAllowedAction("check_run", "created"), false); +}); + +test("refreshes on commit statuses, which carry no action", () => { + assert.equal(isAllowedAction("status", ""), true); + assert.equal(isAllowedAction("pull_request", ""), false); +}); + +test("ignores check runs reported by apps other than code scanning", () => { + assert.equal(isRedundantCheckRunEvent("check_run", { + check_run: { app: codeScanningApp }, + }), false); + assert.equal(isRedundantCheckRunEvent("check_run", { + check_run: { app: { id: 15368 } }, + }), true); + assert.equal(isRedundantCheckRunEvent("check_suite", { + check_suite: { app: { id: 15368 } }, + }), false); +}); + +test("ignores statuses reported on the default branch", () => { + const repository = { default_branch: "main" }; + assert.equal(isDefaultBranchStatusEvent("status", { + repository, + branches: [{ name: "main" }], + }), true); + assert.equal(isDefaultBranchStatusEvent("status", { + repository, + branches: [{ name: "renovate/kotlin-plugin-updates" }], + }), false); + // A fork head is not a branch in the repository that emitted the event. + assert.equal(isDefaultBranchStatusEvent("status", { repository, branches: [] }), false); +}); + +test("reports the head commit when a check event has no pull request", () => { + // Check suites on a fork head report head_branch: null and no pull requests. + const payload = { + repository: { url: "https://api.github.com/repos/open-telemetry/example", default_branch: "main" }, + check_suite: { head_branch: null, head_sha: headSha, pull_requests: [] }, + }; + assert.equal(extractPullRequestNumber("check_suite", payload), undefined); + assert.equal(extractHeadSha("check_suite", payload), headSha); +}); + +test("prefers the pull request a check event names", () => { + const payload = { + repository: { url: "https://api.github.com/repos/open-telemetry/example" }, + check_run: { + head_sha: headSha, + pull_requests: [ + { number: 19286, url: "https://api.github.com/repos/open-telemetry/example/pulls/19286" }, + ], + }, + }; + assert.equal(extractPullRequestNumber("check_run", payload), 19286); +}); + +test("reports the head commit of a status event", () => { + assert.equal(extractHeadSha("status", { sha: headSha }), headSha); + assert.equal(extractHeadSha("status", { sha: "not-a-sha" }), ""); + assert.equal(extractHeadSha("pull_request", { pull_request: { number: 1 } }), ""); +}); + test("recognizes comments performed by the dashboard app", () => { assert.equal(isDashboardSelfTriggeredCommentEvent("issue_comment", { comment: { diff --git a/.github/workflows/pull-request-dashboard-repo.yml b/.github/workflows/pull-request-dashboard-repo.yml index 5946510415e..56be12fc9dd 100644 --- a/.github/workflows/pull-request-dashboard-repo.yml +++ b/.github/workflows/pull-request-dashboard-repo.yml @@ -79,6 +79,7 @@ jobs: permission-issues: read permission-members: read permission-pull-requests: read + permission-statuses: read - name: Restore per-PR classification cache if: inputs.pr_number != '' diff --git a/.github/workflows/pull-request-dashboard.yml b/.github/workflows/pull-request-dashboard.yml index 14c951bdf93..7db9699318a 100644 --- a/.github/workflows/pull-request-dashboard.yml +++ b/.github/workflows/pull-request-dashboard.yml @@ -13,6 +13,10 @@ on: description: Pull request number to refresh. Empty means backfill. required: false type: string + head_sha: + description: Head commit to resolve to a pull request when the event carries no number. + required: false + type: string trigger_event: description: Event that requested the refresh. required: false @@ -30,7 +34,7 @@ permissions: # review id. concurrency: group: >- - pull-request-dashboard-${{ inputs.repository || 'all-repositories' }}-${{ inputs.pr_number || 'backfill' }}-${{ github.event_name == 'workflow_dispatch' && (inputs.trigger_event == '' || inputs.trigger_event == 'workflow_dispatch') && 'manual' || 'refresh' }} + pull-request-dashboard-${{ inputs.repository || 'all-repositories' }}-${{ inputs.pr_number || inputs.head_sha || 'backfill' }}-${{ github.event_name == 'workflow_dispatch' && (inputs.trigger_event == '' || inputs.trigger_event == 'workflow_dispatch') && 'manual' || 'refresh' }} cancel-in-progress: false env: @@ -40,7 +44,7 @@ jobs: resolve-targets: # Targeted refreshes go straight to the reusable workflow, which resolves # its own configuration. Only backfills need a matrix resolved up front. - if: inputs.repository == '' || inputs.pr_number == '' + if: inputs.head_sha == '' && (inputs.repository == '' || inputs.pr_number == '') runs-on: ubuntu-latest permissions: contents: read @@ -63,7 +67,7 @@ jobs: [[ -n "$TARGET_REPOSITORY_FROM_INPUT" ]] || { echo "repository is required when pr_number is set"; exit 1; } fi if [[ -n "$TRIGGER_EVENT_FROM_INPUT" ]]; then - [[ "$TRIGGER_EVENT_FROM_INPUT" =~ ^(schedule|workflow_dispatch|check_suite|pull_request|issue_comment|pull_request_review|pull_request_review_comment|pull_request_review_thread)$ ]] \ + [[ "$TRIGGER_EVENT_FROM_INPUT" =~ ^(schedule|workflow_dispatch|check_suite|check_run|status|pull_request|issue_comment|pull_request_review|pull_request_review_comment|pull_request_review_thread)$ ]] \ || { echo "bad trigger event: $TRIGGER_EVENT_FROM_INPUT"; exit 1; } fi @@ -88,6 +92,70 @@ jobs: echo "matrix={\"include\":${matrix}}" >> "$GITHUB_OUTPUT" echo "$matrix" | jq . + resolve-head-sha: + # Check and status events for a fork head carry no pull request number, so + # the head commit has to be resolved before the reusable workflow runs. + if: inputs.head_sha != '' && inputs.pr_number == '' + runs-on: ubuntu-latest + permissions: + contents: read + environment: protected + outputs: + pr_number: ${{ steps.trigger.outputs.pr_number }} + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Resolve trigger inputs + id: targets + env: + TARGET_REPOSITORY: ${{ inputs.repository }} + HEAD_SHA_FROM_INPUT: ${{ inputs.head_sha }} + run: | + set -euo pipefail + [[ -n "$TARGET_REPOSITORY" ]] || { echo "repository is required when head_sha is set"; exit 1; } + [[ "$HEAD_SHA_FROM_INPUT" =~ ^[0-9a-f]{40}$ ]] || { echo "bad head SHA: $HEAD_SHA_FROM_INPUT"; exit 1; } + name=$(jq -r --arg repo "$TARGET_REPOSITORY" ' + [ .[] | select(.name == $repo) ] | first | .name // empty + ' "$DASHBOARD_CONFIG") + if [[ -z "$name" ]]; then + echo "no configured repository matched: $TARGET_REPOSITORY" >&2 + exit 1 + fi + echo "repository=$name" >> "$GITHUB_OUTPUT" + + - uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + id: head-sha-token + with: + client-id: ${{ vars.PR_DASHBOARD_CLIENT_ID }} + private-key: ${{ secrets.PR_DASHBOARD_PRIVATE_KEY }} + owner: open-telemetry + repositories: ${{ steps.targets.outputs.repository }} + permission-pull-requests: read + + - name: Resolve head commit + id: trigger + env: + HEAD_SHA: ${{ inputs.head_sha }} + REPO_NAME: ${{ steps.targets.outputs.repository }} + GH_TOKEN: ${{ steps.head-sha-token.outputs.token }} + run: | + set -euo pipefail + matches=$(gh api "repos/open-telemetry/${REPO_NAME}/commits/${HEAD_SHA}/pulls" \ + | jq -r --arg sha "$HEAD_SHA" '[ .[] | select(.state == "open" and .head.sha == $sha) | .number ] | sort | join(" ")') + pr_number="${matches%% *}" + if [[ -z "$pr_number" ]]; then + echo "no open pull request has head $HEAD_SHA; skipping refresh." + elif [[ "$matches" == *" "* ]]; then + # One head branch can be open against several base branches. The + # rest keep refreshing on their own events and on the backfill. + echo "head $HEAD_SHA belongs to open pull requests $matches; refreshing #$pr_number" + else + echo "resolved head $HEAD_SHA to pull request #$pr_number" + fi + echo "pr_number=$pr_number" >> "$GITHUB_OUTPUT" + run-repo-dashboard: needs: resolve-targets permissions: @@ -121,6 +189,22 @@ jobs: PR_DASHBOARD_PRIVATE_KEY: ${{ secrets.PR_DASHBOARD_PRIVATE_KEY }} SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} + run-head-sha-dashboard: + needs: resolve-head-sha + # A head commit that matches no open pull request is not a request to + # refresh the whole repository. + if: needs.resolve-head-sha.outputs.pr_number != '' + permissions: + contents: write + copilot-requests: write + uses: ./.github/workflows/pull-request-dashboard-repo.yml + with: + repository: ${{ inputs.repository }} + pr_number: ${{ needs.resolve-head-sha.outputs.pr_number }} + secrets: + PR_DASHBOARD_PRIVATE_KEY: ${{ secrets.PR_DASHBOARD_PRIVATE_KEY }} + SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK_URL }} + notify-hourly-failure: needs: - resolve-targets