open-telemetry · trask · Jul 30, 2026 · Jul 30, 2026 · Jul 30, 2026 · Jul 30, 2026
@@ -172,6 +172,22 @@ the implementation understandable and operationally cheap.
 - Classic branch-protection required status checks are not discovered when they
   have not reported. This is an accepted limitation because configured
   OpenTelemetry repositories use rulesets for required status checks.
+- The rollup is read from the PR's last commit, which can still be the previous
+  head just after a push. That commit's checks are already complete, so they
+  would read as a settled result for code that is no longer proposed. The
+  rollup therefore carries its commit oid and is discarded when it does not
+  match the PR head, leaving check facts unavailable until the rollup catches
+  up.
+- A required context is only pending while the app that owns it may still
+  report. Check suites for the head commit are consulted, and a context is
+  dropped from the pending set once every suite its app created has completed,
+  because the app has then reported everything it is going to. Without this a
+  ruleset context that no workflow produces — an obsolete or conditionally
+  skipped job — would be reported as permanently pending. Such a PR cannot
+  merge either way; the difference is that the dashboard stops treating it as
+  "still running." The suites are read only when an app-owned required context
+  has not reported, so the refreshes that cannot use the answer do not pay for
+  it.
 - A `code_scanning` ruleset rule holds the merge on a check that the code
   scanning app publishes per configured tool, named after that tool, which
   GitHub never marks as required. Those checks are matched by app and by the
@@ -186,9 +202,43 @@ the implementation understandable and operationally cheap.
   Repository-configured `non_blocking_check_patterns` identify failed optional
   checks in a note alongside this action, without changing required-check facts
   or routing.
+- A PR does not advance toward merge while the required checks are unsettled:
+  an author waiting on CI keeps the PR, and a PR already with approvers is not
+  handed to maintainers to merge. Clearing the checks is the author's job, so
+  an outstanding one is not yet a reason to spend anyone else's attention, and a
+  push clears the failing count before the replacement checks produce a result,
+  so the PR would otherwise move forward on evidence that does not exist yet and
+  move back minutes later when the same check fails. Moving back toward the
+  author is never held, because a failing check or new author-owned discussion
+  is evidence the gates cannot undo. Unavailable check results hold the handoff
+  for the same reason a pending one does, and resolve on a later run.
+- A held PR is presented as waiting on its author rather than on the robot it
+  is waiting for, so a separate route would add a section that nobody is
+  expected to act on. What it waits for is named in the columns instead: the CI
+  column already shows running checks, and an outstanding Copilot review is
+  listed in the reviewers column with the same pending icon. Copilot otherwise
+  joins that column only once it has reviewed, so without this the Copilot 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.
+- A held route also holds its wait age. Recomputing it would read the push as
+  the end of the CI failure and fall back to the last approver activity, which
+  is usually far older, so a PR the author had just pushed to would sort to the
+  top of the waiting-on-authors section as the stalest item on the board.
+- A held PR sends no reminder in either direction. The author nudge and the
+  reviewer Slack notification both ask a person to respond, and while the hold
+  lasts the response is owed by a robot that is already working. The author's
+  waiting episode ends when the hold begins, so a later handoff back to the
+  author starts a fresh one instead of resuming a wait the author has answered.
+- While a PR stays on a route where someone other than the author owes it a
+  response, its wait age only moves back, never forward. The fallback for those
+  routes is the last author activity, so a push would otherwise restart the
+  clock and present a review nobody has done in a week as brand new. A handoff
+  from the author route does start a fresh wait, because that push is what put
+  the PR in front of reviewers.
 - Maintenance-bot PRs retain maintainer-oriented routing because the bot cannot
   respond to a dashboard action. Pending required checks affect the CI column
-  but do not change who owns the next action.
+  but never route one of these PRs to its author: a bot PR whose handoff is
+  held waits on reviewers instead.
 
 ## Copilot Review Gate
 
@@ -199,7 +249,7 @@ the implementation understandable and operationally cheap.
 - The setting lists the base branches to gate rather than a single on/off
   switch, because automatic Copilot review is itself configured per branch
   (often only the default branch). Gating a branch with no automatic review
-  would park every ready PR on the copilot route waiting for a review that never
+  would hold every ready PR with its author waiting for a review that never
   runs, so only branches with automatic review are listed and PRs targeting
   other branches route normally.
 - Copilot findings normally return a PR to the author through ordinary
@@ -214,11 +264,26 @@ the implementation understandable and operationally cheap.
   gate or produces fresh actionable threads that route the PR back to the
   author, so re-requesting an unchanged commit is self-correcting rather than a
   re-request loop.
+- An outstanding Copilot review holds the PR where it is the same way an
+  unsettled required check does, including when a reviewer-routing override
+  asked for the handoff. 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 gate withholds the re-review request until the required checks have
+  settled. A route computed while checks are still running is provisional: a
+  failure that has not completed yet cannot route the PR to its author, so the
+  PR looks ready for reviewers and the gate would spend a Copilot review on code
+  CI is about to reject. Unavailable check results are treated the same as
+  running ones, because both mean the routing decision cannot be trusted yet.
+- Delivery re-validates the required checks against live data rather than
+  relying on the routing fingerprint alone. The fingerprint only detects
+  change, so checks that were unsettled when the request was recorded and are
+  still unsettled at delivery would otherwise pass through unnoticed.
 - "Clean" means no inline comments on the current head, counted from the
   review, not from the classifier's actionability judgment. Accepted
   limitation: if Copilot leaves comments the classifier treats as
   non-actionable while they stay unresolved, routing sits at reviewers but the
-  gate holds the PR on the copilot route and re-requests until Copilot returns a
+  handoff stays held with the author and re-requests until Copilot returns a
   comment-free review or the author pushes. The strict count is intentional —
   the gate is a conservative "Copilot had nothing to say about this exact code"
   check, and folding in classifier judgment could let a real-but-non-actionable

@@ -104,6 +104,16 @@ def fetch_current_pr_routing_inputs(
     return raw["pr"], raw
 
 
+def waiting_on_author(result: dict[str, Any] | None) -> bool:
+    # A held PR shows the author route only because a robot gate has not
+    # reported yet, and the author has nothing to answer while it runs.
+    facts = (result or {}).get("facts") or {}
+    return (
+        (result or {}).get("route") == "author"
+        and not facts.get("route_held_for_gates")
+    )
+
+
 def plan_nudge(
     result: dict[str, Any] | None,
     previous: dict[str, Any] | None,
@@ -116,7 +126,7 @@ def plan_nudge(
         or result.get("route") in ("transient-failure", "unknown")
     ):
         return False, entry or None
-    if not result or result.get("route") != "author":
+    if not waiting_on_author(result):
         return False, None
     if nudged_at:
         return False, entry
@@ -252,7 +262,7 @@ def deliver_prepared_author_nudges(
             continue
         pr_number = int(key)
         result = dashboard_prs.get(key)
-        if not result or result.get("route") != "author":
+        if not waiting_on_author(result):
             _due, reset_entry = plan_nudge(result, entry, now)
             if reset_entry is None:
                 updated.pop(key, None)

@@ -17,18 +17,16 @@
     request_copilot_review,
 )
 from state import load_copilot_review_requests, save_copilot_review_requests
-from utils import actor_login, format_ts
-
-
-_COPILOT_REVIEWER_LOGINS = {
-    "copilot",
-    "copilot-pull-request-reviewer",
-    "copilot-pull-request-reviewer[bot]",
-}
+from utils import (
+    actor_login,
+    format_ts,
+    is_copilot_reviewer_login,
+    required_checks_settled,
+)
 
 
 def is_copilot_reviewer(obj: dict[str, Any] | None) -> bool:
-    return actor_login(obj).lower() in _COPILOT_REVIEWER_LOGINS
+    return is_copilot_reviewer_login(actor_login(obj))
 
 
 def copilot_review_status(
@@ -58,22 +56,31 @@ def copilot_review_status(
     return True, bool(latest_review.get("finding_count"))
 
 
-def apply_copilot_review_gate(
+def copilot_review_outstanding(facts: dict[str, Any], *, enabled: bool) -> bool:
+    if not enabled:
+        return False
+    return not facts.get("copilot_review_exists") or bool(
+        facts.get("copilot_review_needed")
+    )
+
+
+def set_copilot_review_request_needed(
     facts: dict[str, Any],
     route: str,
     *,
     enabled: bool,
-) -> str:
-    facts["copilot_review_request_needed"] = False
-    if not enabled or route not in ("approver", "maintainer"):
-        return route
-    if not facts.get("copilot_review_exists"):
-        return "copilot"
-    if not facts.get("copilot_review_needed"):
-        return route
-    if not facts.get("copilot_review_requested"):
-        facts["copilot_review_request_needed"] = True
-    return "copilot"
+) -> None:
+    # Requesting a re-review before the checks report would spend it on code CI
+    # is about to reject, and a PR GitHub has never reviewed is already queued
+    # for the automatic first review.
+    facts["copilot_review_request_needed"] = (
+        enabled
+        and route in ("approver", "maintainer")
+        and bool(facts.get("copilot_review_exists"))
+        and bool(facts.get("copilot_review_needed"))
+        and not facts.get("copilot_review_requested")
+        and required_checks_settled(facts)
+    )
 
 
 def record_copilot_review_observation(
@@ -89,7 +96,6 @@ def record_copilot_review_observation(
     if (
         not result
         or result.get("failed")
-        or result.get("route") != "copilot"
         or not facts.get("copilot_review_request_needed")
         or not head_sha
         or not routing_fingerprint
@@ -105,6 +111,13 @@ def record_copilot_review_observation(
     save_copilot_review_requests(requests)
 
 
+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)
+
+
 def stale_request_reason(
     entry: dict[str, Any],
     pr: dict[str, Any],
@@ -121,6 +134,17 @@ def stale_request_reason(
             f"head is {current_head or '(missing)'} "
             f"but {entry.get('head_sha') or '(missing)'} was observed"
         )
+    # The fingerprint below only detects change, so checks that were unsettled
+    # when the request was recorded and still are would otherwise pass through.
+    checks = raw.get("checks")
+    if checks is None:
+        return "required check results are unavailable"
+    failing = [check for check in checks if check.get("bucket") in ("fail", "cancel")]
+    if failing:
+        return f"required checks are failing: {named_checks(failing)}"
+    pending = [check for check in checks if check.get("bucket") == "pending"]
+    if pending:
+        return f"required checks have not completed: {named_checks(pending)}"
     if not entry.get("routing_input_fingerprint"):
         return "no routing fingerprint was observed"
     if current_routing_fingerprint != entry["routing_input_fingerprint"]: