Do not advance a PR until CI and Copilot report

[trask]

A PR whose required checks have not reported yet can look ready for reviewers. route_pr only sees ci_failing_count, and a push clears that count minutes before the replacement checks produce a result, so the PR leaves the author on evidence that does not exist yet — and the Copilot review gate then spends a re-review on code CI is about to reject. That is what happened on opentelemetry-java-instrumentation#19334: the head commit was pushed at 04:25:12, the dashboard requested a Copilot review at 04:26:19, and the first required check failure completed at 04:28:35.

The required checks and the Copilot review are gates the author owns, so while one is outstanding the PR does not advance: an author waiting on CI keeps it, and a PR already with approvers is not handed to maintainers to merge. Movement back toward the author is never held, because a failing check or a newly opened discussion is evidence the gates cannot undo. The gates run last, after the manual routing override, because the override says the author is done with the discussion, not that the robots have finished, and a handoff made before they report would only change back. The override acknowledgement says what the handoff is waiting for.

This retires the copilot route. A PR waiting on an automated review was never actionable by anyone in that section, and the same PR waiting on CI was already shown as waiting on its author, so the two robot gates are now the same state with the same owner. What the PR waits for is named in the columns instead: the CI column already showed running checks, and an outstanding Copilot review is now listed in the reviewers column with the same pending icon — Copilot otherwise joins that column only once it has reviewed, so the gate would hold a PR with nothing on the row to explain why. The live status comment tells the author the handoff happens once both are clean. Stored results carrying the retired route degrade to "not yet advanced", which is exactly the state the hold expects, so no state version bump is needed.

The wait age needed the same protection on the other side of the handoff. A held PR keeps the wait it already had, because recomputing it would read the push as the end of the CI failure and fall back to the last approver activity — often weeks earlier — sorting a freshly pushed PR to the top of the waiting-on-authors section. And while a PR stays on a route where someone other than the author owes it a response, its age now only moves back, never forward: the fallback for those routes is the last author activity, so a typo fix would otherwise present a review nobody has done in a week as brand new. A handoff from the author still starts a fresh wait, because that push is what put the PR in front of reviewers.

Two check-reading bugs had to be fixed first for the hold to terminate. commits(last: 1) can still return the previous head just after a push, and that commit's checks are complete, so the rollup now carries its commit oid and is discarded when it does not match the PR head. And a ruleset context that no workflow produces — an obsolete or conditionally skipped job — was reported as permanently pending, which would have held such a PR forever; check suites for the head commit are now consulted, across every page of that endpoint, and a context is dropped from the pending set once every suite its app created has completed. A scan of all 17 configured repositories found this affects four of them, including three contexts on opentelemetry-collector-contrib.

[Copilot]

Pull request overview

Prevents premature reviewer routing and Copilot review requests while required CI checks remain unsettled.

Changes:

• Holds author routing and wait age until checks report.
• Validates check data against the current head and check suites.
• Revalidates CI before delivering Copilot review requests.

Reviewed changes

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

Show a summary per file

File	Description
state.py	Bumps persisted-state versions.
github_cli.py	Adds head and suite-aware check handling.
dashboard.py	Adds author-route and wait-age holds.
copilot_review.py	Gates review requests on CI state.
RATIONALE.md	Documents the new routing behavior.
test_state.py	Updates version assertions.
test_github_cli.py	Tests check-rollup and suite behavior.
test_dashboard.py	Tests route and wait-age holds.
test_copilot_review.py	Tests delivery-time CI validation.
test_author_nudge.py	Updates check-rollup fixtures.

---

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

[opentelemetry-pr-dashboard]

Pull request dashboard status

Waiting on the author · refreshed 2026-07-30 22:02 UTC

Move out of draft to request review.

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]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (5)

.github/scripts/pull-request-dashboard/dashboard.py:1344

• Returning author also makes this result eligible for author reminders: author_nudge.plan_nudge checks only route == "author", and the preserved wait can make a reminder immediately due. That reminder says items need attention and asks for discussion replies even though a gate-held PR's status tells the author to wait. Suppress or pause author-nudge planning and delivery while route_held_for_gates is true.

    facts["route_held_for_gates"] = held
    return "author" if held else route

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

• Each contexts page is fetched by a separate GraphQL request. If commits(last: 1) changes from the stale head to the current head between pages, this overwrites head_oid while checks_by_identity still contains contexts from the stale page; the later head comparison then passes with a mixed rollup. Reject the rollup whenever a nonempty OID differs from the first page's OID.

            head_oid = commit.get("oid") or head_oid

.github/scripts/pull-request-dashboard/dashboard.py:1334

• This hold also applies to maintenance-bot PRs, so a newly observed bot PR with pending checks is returned as author. That conflicts with route_pr, which deliberately never assigns these bots an author action, and with RATIONALE.md:227-229, which says pending checks must not change their maintainer-oriented ownership. Exclude maintenance bots from the hold.

This issue also appears on line 1343 of the same file.

    held = (
        route in REVIEWER_ROUTES
        and (previous_result or {}).get("route") not in REVIEWER_ROUTES

.github/scripts/pull-request-dashboard/pr_status_comment.py:220

• A hold can be caused only by pending CI when the Copilot gate is disabled, or only by Copilot after CI has settled. This unconditional sentence therefore tells authors to wait for a review that may not be required or for checks that already finished. Render the actual outstanding gate(s), or use accurate generic wording.

    if held_for_gates:
        return [
            "Wait for the required status checks and the Copilot review to "
            "finish; this pull request moves to reviewers once they are clean."

.github/scripts/pull-request-dashboard/dashboard_override.py:238

• The override can be held solely for checks or solely for Copilot, but this acknowledgement always claims both are outstanding. In particular, branches with the Copilot gate disabled may never receive such a review, leaving a misleading permanent instruction. Pass the actual hold reasons into the reply or use gate-neutral wording.

        if reply.get("held_for_gates"):
            message = (
                "accepted the reviewer-routing override; the reviewer handoff "
                "is waiting on the required status checks and the Copilot "
                "review."

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (4)

.github/scripts/pull-request-dashboard/state.py:26

• Bumping this version discards all stored PR results (load_state_file rejects mismatched versions), including the previous route and wait age that the new hold relies on. On deployment, unsettled PRs will therefore default from author and cannot preserve their existing handoff or wait; this also contradicts the PR description's explicit statement that retired copilot routes degrade correctly without a state-version bump. Keep the dashboard version compatible so the migration can use the stored evidence.

DASHBOARD_STATE_VERSION = 7

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

• A lagging commits(last: 1) can catch up between context pages. This assignment then overwrites the first page's old OID with the current OID, so the final comparison succeeds even though checks_by_identity contains contexts from two commits. Detect an OID change across pages and return unavailable data instead of combining the pages.

            head_oid = commit.get("oid") or head_oid

.github/scripts/pull-request-dashboard/dashboard.py:1361

• When an approver PR is held from advancing to maintainer, this returns approver with route_held_for_gates=True. notifications.py:215 treats every approver route as actionable and ignores this flag, so due Slack reminders still ping reviewers while CI or Copilot is outstanding—the human attention this gate is intended to avoid. Suppress notifications for held results and cover the approver-to-maintainer hold case.

    facts["route_held_for_gates"] = held
    return previous_route if held else route

.github/scripts/pull-request-dashboard/dashboard.py:1358

• This CI hold also applies to maintenance-bot PRs when they advance (including every first observation, where the previous route defaults to author). That bypasses route_pr's maintenance-bot exception and places a bot-owned PR in the author section while checks run, although the rationale states pending checks must not change its human owner. Exempt maintenance bots from the required-check arm of this hold.

            not required_checks_settled(facts)

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (4)

.github/scripts/pull-request-dashboard/dashboard.py:1359

• Maintenance-bot PRs are also held here. On their first observation, or when moving from approver to maintainer, pending CI makes the route fall back to author, even though these bots cannot act there and the documented behavior in RATIONALE.md:231-233 says pending checks must not change their owner. Exempt maintenance bots from the CI-pending branch of this hold (while preserving any separately intended Copilot gate).

    held = (
        route_progress(route) > route_progress(previous_route)
        and (
            not required_checks_settled(facts)
            or facts["copilot_review_outstanding"]
        )

.github/scripts/pull-request-dashboard/pr_status_comment.py:220

• held_for_gates does not mean both gates are outstanding: for a branch where the Copilot gate is disabled, pending CI alone sets it, and when CI is complete a Copilot review alone can set it. This message therefore tells authors to wait for work that may already be complete or not configured. Render the actual outstanding gate(s) using ci_pending_count/check availability and copilot_review_outstanding.

    if held_for_gates:
        return [
            "Wait for the required status checks and the Copilot review to "
            "finish; this pull request moves to reviewers once they are clean."

.github/scripts/pull-request-dashboard/dashboard_override.py:238

• The aggregate hold flag can represent only pending CI or only Copilot, but this acknowledgement always names both. In particular, overrides on branches without a configured Copilot gate will incorrectly say the handoff is waiting on Copilot. Include the individual gate states in the reply and name only those that are outstanding.

        if reply.get("held_for_gates"):
            message = (
                "accepted the reviewer-routing override; the reviewer handoff "
                "is waiting on the required status checks and the Copilot "
                "review."

.github/scripts/pull-request-dashboard/render.py:154

• This only recognizes the two verbose Copilot logins mapped by reviewer_display_name, while copilot_review.is_copilot_reviewer also accepts copilot. If a review under that supported login has findings, the existing reviewer entry is not marked pending and a second synthetic Copilot row is appended. Add the supported short login to the display-name aliases (or share the reviewer predicate).

        if reviewer_display_name(reviewer.get("login") or "") == "Copilot":
            reviewer["pending_review"] = True

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (3)

.github/scripts/pull-request-dashboard/state.py:34

• This changes the rendered status comment for existing PRs, but only the Copilot request-state version is bumped. The convention immediately below (state.py:37-39) requires incrementing STATUS_COMMENT_REVISION when existing comments need new behavior; without that rollout, old comments can keep the retired “Waiting on Copilot”/old next step until another refresh happens.

COPILOT_REVIEW_REQUEST_STATE_VERSION = 4

.github/scripts/pull-request-dashboard/dashboard.py:1367

• A held human PR is returned as route == "author", but the author-nudge path still treats every author route as actionable (author_nudge.py:119-130) and renders “waiting on you”/“items that need your attention.” If CI or Copilot remains outstanding long enough—or an existing author episode becomes due during the hold—the author receives a misleading nudge while there is nothing to do. Suppress/reset author nudges when route_held_for_gates is true.

    facts["route_held_for_gates"] = held
    return previous_route if held else route

.github/scripts/pull-request-dashboard/test_notify_slack.py:19

• This fixture uses route == "author", so it passes solely because next_notifications already ignores every non-approver route. It misses the new approver→maintainer hold: that PR remains on approver with route_held_for_gates == True, and next_notifications can still send Slack reminders to listed unapproved reviewers even though review is complete and only a robot gate is outstanding. Skip held results in notification delivery and cover an approver fixture here.

                "route": "author",
                "facts": {
                    "route_held_for_gates": True,

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (6)

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

• The return type annotation for gh_pr_check_rollup is now incorrect: the function returns a dict containing both strings (head_oid) and lists of dicts. Update the annotation to reflect the actual shape (e.g., a TypedDict for the rollup, or dict[str, Any] | None) so type checking and future refactors don’t rely on an invalid contract.

    repo: str,
    pr_id: str,
    non_blocking_check_patterns: list[str],
) -> dict[str, list[dict[str, Any]]] | None:

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

• The return type annotation for gh_pr_check_rollup is now incorrect: the function returns a dict containing both strings (head_oid) and lists of dicts. Update the annotation to reflect the actual shape (e.g., a TypedDict for the rollup, or dict[str, Any] | None) so type checking and future refactors don’t rely on an invalid contract.

    return {
        "head_oid": head_oid,
        "required": [check for check, is_required in checks if is_required],
        "non_blocking_failures": [
            check
            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()),
    }

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

• settled_check_suite_app_ids adds an extra REST call per PR refresh, even when it can’t affect the result (e.g., no required-status-check rules, no required contexts with integration_id, or when check results are already None). To reduce API usage/rate-limit pressure, conditionally fetch settled app IDs only when needed (for example: only when required_check_contexts(branch_rules) includes at least one entry with a non-None integration_id and some required contexts are missing from the rollup).

        settled_app_ids_future = pool.submit(
            settled_check_suite_app_ids,
            repo,
            pr.get("headRefOid") or "",
        )

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

• settled_check_suite_app_ids adds an extra REST call per PR refresh, even when it can’t affect the result (e.g., no required-status-check rules, no required contexts with integration_id, or when check results are already None). To reduce API usage/rate-limit pressure, conditionally fetch settled app IDs only when needed (for example: only when required_check_contexts(branch_rules) includes at least one entry with a non-None integration_id and some required contexts are missing from the rollup).

        checks = include_missing_required_checks(
            None if check_rollup is None else check_rollup["required"],
            required_check_contexts(branch_rules),
            settled_app_ids_future.result(),
        )

.github/scripts/pull-request-dashboard/render.py:152

• display_reviewers constructs dict(reviewer) for every entry in facts['reviewers']. If any entry is not already a mapping (e.g., an unexpected string/None from upstream data), this will raise and break dashboard rendering. Consider filtering to isinstance(reviewer, dict) before copying, or using a safer copy strategy that preserves existing dict reviewers while skipping/ignoring malformed entries.

def display_reviewers(facts: dict[str, Any]) -> list[dict[str, Any]]:
    # Copilot only joins the reviewer list once it has reviewed, so an
    # outstanding review has to be added for the wait to be visible at all.
    reviewers = [dict(reviewer) for reviewer in facts.get("reviewers") or []]
    if not facts.get("copilot_review_outstanding"):
        return reviewers

.github/scripts/pull-request-dashboard/copilot_review.py:128

• named_checks includes empty strings for checks missing a name, which can produce unclear messages like required checks are failing: , , build. Filter out falsy names (and optionally de-duplicate) before sorting/joining so stale-request reasons remain readable even if upstream check objects are partially populated.

def named_checks(checks: list[dict[str, Any]]) -> str:
    names = sorted(check.get("name") or "" for check in checks)
    if len(names) > 3:
        return f"{', '.join(names[:3])} and {len(names) - 3} more"
    return ", ".join(names)

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (3)

.github/scripts/pull-request-dashboard/copilot_review.py:128

• named_checks() can emit awkward messages if a check dict is missing/empty for name (leading commas/blank entries) and can repeat the same name multiple times. Consider filtering out falsey names and de-duplicating before formatting so stale-request reasons remain readable and stable.

def named_checks(checks: list[dict[str, Any]]) -> str:
    names = sorted(check.get("name") or "" for check in checks)
    if len(names) > 3:
        return f"{', '.join(names[:3])} and {len(names) - 3} more"
    return ", ".join(names)

.github/scripts/pull-request-dashboard/render.py:156

• Detecting Copilot by comparing the display name string (== "Copilot") couples logic to presentation and makes future name/presentation tweaks risky. Prefer checking the underlying login against an explicit allowlist (e.g., the mapped Copilot logins) so “is Copilot reviewer” stays independent from how it’s rendered.

    for reviewer in reviewers:
        if reviewer_display_name(reviewer.get("login") or "") == "Copilot":
            reviewer["pending_review"] = True
            return reviewers

.github/scripts/pull-request-dashboard/pr_status_comment.py:225

• This message says “finish”, but required_checks_settled is also false when check results are unavailable (not just running). Consider wording that covers both cases (e.g., “to settle” / “to report”) so authors aren’t told checks are “finishing” when the system currently can’t read results.

    if held_gates:
        return [
            f"Wait for {held_gates} to finish; this pull request moves to "
            "reviewers once the results are clean."
        ]

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (3)

.github/scripts/pull-request-dashboard/render.py:156

• display_reviewers() assumes every entry in facts['reviewers'] is a mapping; dict(reviewer) will raise TypeError if the list contains a non-dict value (other code in this repo guards with isinstance(reviewer, dict)). Filter to dicts (and/or coerce via a safe default) before copying to make the renderer robust to unexpected API payload shapes.

def display_reviewers(facts: dict[str, Any]) -> list[dict[str, Any]]:
    # Copilot only joins the reviewer list once it has reviewed, so an
    # outstanding review has to be added for the wait to be visible at all.
    reviewers = [dict(reviewer) for reviewer in facts.get("reviewers") or []]

.github/scripts/pull-request-dashboard/render.py:147

• REVIEWER_DISPLAY_NAMES is keyed by lowercased Copilot logins, but reviewer_display_name() looks up the raw login. This can miss the friendly name for non-canonical casing (even though other logic normalizes casing via is_copilot_reviewer_login). Consider normalizing login (e.g., .strip().lower()) for the lookup while still returning the original login when there is no match.

def reviewer_display_name(login: str) -> str:
    return REVIEWER_DISPLAY_NAMES.get(login, login)

.github/scripts/pull-request-dashboard/copilot_review.py:121

• named_checks() includes empty strings when a check dict lacks a name, which can yield unclear outputs like ', , build' in stale_request_reason() messages. Filter out falsy names before sorting/joining to keep diagnostic text readable.

def named_checks(checks: list[dict[str, Any]]) -> str:
    names = sorted(check.get("name") or "" for check in checks)
    if len(names) > 3:
        return f"{', '.join(names[:3])} and {len(names) - 3} more"
    return ", ".join(names)

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (3)

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

• Returning an empty set on check-suites read failure conflates “no apps are settled” with “unknown”, which can incorrectly keep obsolete/never-reporting required contexts in the pending set until the API read succeeds (potentially holding PRs longer than necessary during repeated transient failures). Consider returning None on exception and treating the check-suite signal as unavailable (e.g., fall back to checks=None / ‘check results unavailable’ semantics) rather than assuming nothing is settled.

def settled_check_suite_app_ids(repo: str, head_sha: str) -> set[int]:
    # An app is settled once every check suite it created for this head has
    # completed, which means it has reported every context it is going to.
    if not head_sha:
        return set()
    try:
        pages = gh_api(
            f"/repos/{repo}/commits/{head_sha}/check-suites?per_page=100",
            paginate=True,
        )
    except RuntimeError:
        return set()

.github/scripts/pull-request-dashboard/copilot_review.py:121

• As written, named_checks() can include empty strings (if any check lacks a name) and can repeat names if duplicates exist, producing awkward gate messages like leading commas or repeated entries. Filtering out falsy names and de-duplicating before formatting would make the resulting error text consistently readable and actionable.

def named_checks(checks: list[dict[str, Any]]) -> str:
    names = sorted(check.get("name") or "" for check in checks)
    if len(names) > 3:
        return f"{', '.join(names[:3])} and {len(names) - 3} more"
    return ", ".join(names)

.github/scripts/pull-request-dashboard/route_presentation.py:66

• The new user-facing phrasing is central to held-route UX, but there’s no direct test asserting the output when both gates are outstanding at once (required checks unsettled and Copilot review outstanding). Adding a unit test for that combined case would protect the exact messaging used by both pr_status_comment and dashboard_override.

def outstanding_gate_phrase(facts: dict[str, Any]) -> str:
    # Only one gate has to be outstanding for a PR to be held, and a branch
    # without the Copilot gate never has that one, so naming both would tell
    # the author to wait for work that is finished or never happens.
    gates = []
    if not facts.get("required_checks_settled"):
        gates.append("the required status checks")
    if facts.get("copilot_review_outstanding"):
        gates.append("the Copilot review")
    return " and ".join(gates)

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

.github/scripts/pull-request-dashboard/state.py:271

• Migrating only the route leaves former copilot records looking like ordinary author-owned work: the cached facts lack route_held_for_gates and copilot_review_outstanding. Until each PR is refreshed, the dashboard omits the pending Copilot indicator, and waiting_on_author() can even treat a prepared author nudge as deliverable. Preserve the gate facts during migration (deriving check settlement from the cached ci_pending_count) so cached records behave like newly computed held routes.

def migrated_pr_record(record: Any) -> Any:
    # The retired copilot route held a PR while its review was outstanding,
    # which is now part of the author's turn.
    if not isinstance(record, dict) or record.get("route") != "copilot":
        return record
    return {**record, "route": "author"}

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

.github/scripts/pull-request-dashboard/state.py:276

• A cached maintenance-bot PR on the retired copilot route is migrated to author, even though maintenance bots deliberately have no actionable author route. Such PRs could previously use the Copilot route, and after this migration both the cached dashboard and the next gate hold will show them under “Waiting on authors” (the hold accepts the migrated author as its previous route). Preserve the maintenance-bot fallback by migrating these records to approver.

        "route": "author",