Skip to content

Refresh required check status for fork pull requests - #201

Open
trask wants to merge 6 commits into
open-telemetry:mainfrom
trask:refresh-checks-on-fork-prs
Open

Refresh required check status for fork pull requests#201
trask wants to merge 6 commits into
open-telemetry:mainfrom
trask:refresh-checks-on-fork-prs

Conversation

@trask

@trask trask commented Jul 30, 2026

Copy link
Copy Markdown
Member

Important

Two GitHub App changes are required before this merges. The target-repository dashboard App must be granted the Commit statuses: read repository permission, which is a new permission, so every installation has to approve it, and it must be subscribed to the Check run and Status webhook events. The permission is not optional or deferrable: the dashboard token step now requests statuses: read, and create-github-app-token fails outright when an installation has not granted a requested permission, so merging before every installation has approved it breaks every dashboard run.

The dashboard reported "Required checks are failing" on pull requests whose checks had all passed. The code scanning app publishes its per-tool check run as NEUTRAL before the analysis is uploaded and then rewrites that check run in place, and since #194 a NEUTRAL code scanning result counts as a required failure, so an analysis that was still running pinned the pull request to its author. That undetermined result is now only reported as failing once every check at the head has finished, including optional ones, because the optional job that uploads the analysis is what decides whether the result is final.

Nothing then told the dashboard that the result had changed. The in-place rewrite leaves the enclosing check suite untouched, and required checks reported as commit statuses, such as EasyCLA, are never part of a check suite at all. The webhook bridge now subscribes to code scanning check runs and to commit statuses. Check runs from other apps are ignored, because their check suite already reports them and one refresh per job would multiply webhook volume, and statuses on the default branch are ignored.

Wiring those events up surfaced a larger gap. GitHub omits the pull request association from check and status events whose head branch lives in a fork: all six check suites on a fork head report no pull request, while all sixteen on a same-repo head report one. Fork pull requests, which are nearly every contribution, therefore never got a refresh when CI finished and depended on the hourly backfill. Events that carry no pull request number now dispatch the head commit instead, and the workflow resolves it to an open pull request. The webhook bridge cannot resolve it itself, because its GitHub App is installed only on shared-workflows.

Measured across all 17 configured repositories before the fix: 1 of the 84 pull requests stored as failing was stale, both stale cases seen were this bug, and stored head SHAs matched live for all 594 open non-draft tracked pull requests.

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Jul 30, 2026

Copy link
Copy Markdown

Pull request dashboard status

Waiting on reviewers · refreshed 2026-07-30 21:26 UTC

Review the latest changes.

Status above doesn't look right?
  • Just replied or pushed? Anything around or after the refresh time above may not be picked up yet — give it a few minutes.
  • Anything look wrong? Report it with what you expected; it helps us improve the dashboard.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds timely required-check refreshes for fork pull requests and improves handling of provisional code-scanning results.

Changes:

  • Dispatches check-run and commit-status webhook events.
  • Resolves fork head SHAs to open pull requests.
  • Defers neutral code-scanning failures while checks remain pending.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
.github/workflows/pull-request-dashboard.yml Resolves head SHAs and dispatches targeted refreshes.
.github/scripts/pull-request-dashboard/WEBHOOK_SETUP.md Documents new permissions and events.
.github/scripts/pull-request-dashboard/test_github_webhook.mjs Tests webhook filtering and SHA extraction.
.github/scripts/pull-request-dashboard/test_github_cli.py Tests pending-check classification.
.github/scripts/pull-request-dashboard/test_dashboard.py Updates rollup fixtures.
.github/scripts/pull-request-dashboard/test_author_nudge.py Updates routing fixtures.
.github/scripts/pull-request-dashboard/RATIONALE.md Explains refresh and classification behavior.
.github/scripts/pull-request-dashboard/netlify/functions/github-webhook.mjs Handles check-run and status events.
.github/scripts/pull-request-dashboard/github_cli.py Tracks pending checks and provisional results.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/scripts/pull-request-dashboard/github_cli.py Outdated
Comment thread .github/workflows/pull-request-dashboard.yml Outdated
Comment thread .github/scripts/pull-request-dashboard/WEBHOOK_SETUP.md

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

.github/scripts/pull-request-dashboard/github_cli.py:410

  • workflow is only the workflow definition's display name, not a workflow-run execution identifier. If the same workflow runs twice for one SHA (for example, a push run plus a manual/reusable invocation), same-named jobs collapse here; a later completed check can therefore hide an earlier still-running check and make the neutral code-scanning result fail before every check has finished. Include the workflow-run execution ID in the identity (while still collapsing rerun attempts of that execution) rather than relying on the workflow name.
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"]

…ork-prs

# Conflicts:
#	.github/scripts/pull-request-dashboard/WEBHOOK_SETUP.md
#	.github/workflows/pull-request-dashboard.yml
@trask
trask requested a review from Copilot July 30, 2026 19:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

.github/workflows/pull-request-dashboard.yml:47

  • If both inputs.head_sha and inputs.pr_number are provided, neither resolve-targets nor resolve-head-sha will run, and the workflow will effectively do nothing without producing a clear validation error. Add an always-running validation step/job (or adjust the conditions) to fail fast when both are set (and optionally document/enforce mutual exclusivity at the input level).
    if: inputs.head_sha == '' && (inputs.repository == '' || inputs.pr_number == '')

.github/workflows/pull-request-dashboard.yml:98

  • If both inputs.head_sha and inputs.pr_number are provided, neither resolve-targets nor resolve-head-sha will run, and the workflow will effectively do nothing without producing a clear validation error. Add an always-running validation step/job (or adjust the conditions) to fail fast when both are set (and optionally document/enforce mutual exclusivity at the input level).
  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 == ''

.github/scripts/pull-request-dashboard/netlify/functions/github-webhook.mjs:137

  • As written, a check_run payload missing check_run.app.id will be treated as redundant and ignored (because undefined !== CODE_SCANNING_APP_ID). That makes the refresh path brittle to payload shape changes or unexpected event variants. Consider only ignoring when app.id is present and explicitly known to be non-code-scanning; if app.id is missing/unknown, dispatch the refresh instead.
export function isRedundantCheckRunEvent(eventName, payload) {
  if (eventName !== "check_run") {
    return false;
  }
  const app = (payload.check_run || {}).app || {};
  return app.id !== CODE_SCANNING_APP_ID;
}

.github/scripts/pull-request-dashboard/github_cli.py:418

  • When workflow_run_id is None (which can happen for checks not tied to a workflow run), different in-flight executions with the same name/integration can collapse into the same execution identity and overwrite each other in latest_by_execution. This can drop entries from the new pending set and mis-drive the checks_still_running signal. Consider falling back to a more specific identifier when workflow_run_id is missing (e.g., include check_run_id for check runs, and/or started_at for status contexts) to avoid accidental collisions.
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"]

@trask
trask marked this pull request as ready for review July 30, 2026 20:09
@trask
trask requested a review from a team as a code owner July 30, 2026 20:09
@trask
trask requested a review from adrielp July 30, 2026 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants