fix(review): reject failed required checks before launching an agent

[neubig]

Problem

The GitHub PR reviewer launches an agent conversation even when GitHub already reports a required check as failed. On 2026-09-22, requesting all-hands-bot on OpenHands/OpenHands#17544 delivered the native review_requested webhook and created reviewer conversation f46343fd-9486-5943-a38f-fd8a279dd40f, although the required Validate PR description check was already failing and the repository workflow had already posted the specific remediation.

This spends an LLM slot on a PR that is deterministically not reviewable and contributes to host pressure. The reviewer should reserve agent work for PRs that pass cheap deterministic eligibility checks.

Desired behavior

Before creating a review conversation, the reviewer evaluates existing deterministic blockers for the current head. A known failing required check stops the run without starting an agent. Once the author pushes or edits the PR and the required checks pass, an explicit new review request can start the normal review.

Acceptance criteria

• The reviewer checks current-head required-check state before creating an agent conversation.
• A completed failing or cancelled required check prevents conversation creation and leaves a concise, non-duplicative explanation of what must be fixed when an equivalent repository workflow comment does not already exist.
• Pending checks do not produce a false approval; the automation either waits within a bounded budget or exits with a clear retry condition without starting the reviewer agent prematurely.
• Checks from an obsolete head do not block the current head.
• A subsequent explicit all-hands-bot review request after the current head is green proceeds through the existing review flow.
• Tests prove that a failing required check creates zero agent conversations and that a green current head creates exactly one.
• Existing review verdict, maintainer-handoff, and profile/secret behavior remains unchanged.

Live evidence

• PR: ci(docker): verify agent server version in image OpenHands#17544
• Native webhook delivery was HTTP 200 and dispatched reviewer run b373576d-2934-4b9a-b41a-cb219de1d7b9.
• The run created conversation f46343fd-9486-5943-a38f-fd8a279dd40f despite the existing failed Validate PR description check; the conversation was paused to free the slot.

---

OpenHands AI triage

The following comments and acceptance criteria were added by the OpenHands AI agent.

Triage

This is a bug report against the deterministic PR reviewer automation in this repository, not the Actions-based plugins/pr-review. Bounded scope: add a pre-dispatch eligibility gate to skills/github-pr-reviewer/scripts/worker.py (the catalog bundle entrypoint python3 worker.py) so PullRequestReviewer.run() evaluates the current head's check runs before calling dispatcher.deliver(...); extend the shared skills/github/scripts/github_client.py transport if a check-run read is needed; cover both the reviewer-request event path and the scheduled label scan; update skills/github-pr-reviewer/SKILL.md / README.md; and bump the automations/catalog/github-pr-reviewer/manifest.json bundle version because shipped files change.

The four design questions raised earlier are resolved by the maintainer's comment and are folded into the criteria below: every completed exact-head check run with failure, cancelled, or timed_out blocks unless its conclusion is neutral or skipped, with no dependence on ruleset/branch-protection visibility and no manually maintained check-name list; any queued or in-progress exact-head check exits without launching an agent and states that review is waiting on checks; remediation comments deduplicate through a stable machine-readable marker carrying the head SHA and gate category; and the gate applies to both event and scheduled modes.

Non-goals: no change to the review prompt, agent behavior, verdict parsing, or the existing maintainer handoff in scripts/maintainer_handoff.py; no change to profile or secret selection; no branch-protection or ruleset dependency; no configured check-name allowlist; no merge or auto-merge behavior; and no change to the Actions-based plugins/pr-review.

Actual Behavior

The reviewer creates a review conversation for a PR whose current head already has a deterministically failing required check. On 2026-09-22, requesting all-hands-bot on OpenHands/OpenHands#17544 delivered the native review_requested webhook (HTTP 200, reviewer run b373576d-2934-4b9a-b41a-cb219de1d7b9) and created conversation f46343fd-9486-5943-a38f-fd8a279dd40f even though the head's Validate PR description check was already failing; the conversation was paused to free the slot. As of this writing the head c49ff9c4350ebe26be4d196d15ecf6b7c34dec08 reports that check as COMPLETED / FAILURE.

Reproduce against the automation's own delivery tests from the repository root:

uv sync --group test
uv run pytest -q tests/test_github_reviewer_delivery.py

The existing cases assert that a trigger dispatches a review unconditionally, so the failing-check case has no coverage today.

Acceptance Criteria

• Before any conversation is created, the reviewer reads the check runs reported for the PR's current head SHA and classifies each as blocking, non-blocking, or pending; this evaluation happens ahead of dispatcher.deliver(...) in PullRequestReviewer.run().
• A completed check run on the current head with conclusion failure, cancelled, or timed_out prevents conversation creation, and no agent conversation is started for that trigger.
• A completed check run with conclusion success, neutral, or skipped does not block the review.
• If any check run for the current head is queued or in_progress, the run exits without launching an agent and reports that the review is waiting on checks, without holding a worker slot to poll.
• A blocked or waiting run does not consume the trigger: a later explicit all-hands-bot review request, or the normal scheduled scan, starts the review once the current head's checks are non-blocking.
• Check runs attributed to a different (obsolete) head SHA do not block the current head.
• When a check blocks the review, the automation leaves exactly one concise explanation of what must be fixed, identified by a stable machine-readable marker that includes the head SHA and the gate category; on a later run for a different head the marked comment is updated or replaced rather than duplicated, and an already-present equivalent repository workflow comment is not duplicated.
• The gate uses the check-run data reachable with the reviewer's existing token and does not require branch-protection or ruleset read access, and it does not depend on a manually maintained list of check names.
• The gate applies identically in the reviewer-request event path and the scheduled label-scan path.
• Automated tests prove that a blocking current-head check produces zero agent conversations and that a green current head produces exactly one, and that a pending check produces zero; uv run pytest -q passes.
• Existing review verdict parsing, the maintainer handoff, and profile/secret selection are unchanged.
• skills/github-pr-reviewer/SKILL.md and README.md describe the eligibility gate and the waiting-on-checks outcome, and automations/catalog/github-pr-reviewer/manifest.json carries a bumped bundle version with regenerated catalog artifacts.

[github-actions]

⚠️ Not yet ready-for-dev

This issue does not yet meet the readiness criteria, so the
ready-for-dev label has not been applied (or has been removed).
The criteria are type-specific:

Bug reports (bug label):

• The ### Actual Behavior section must describe reproducible steps
  and include a command such as uv run pytest, pytest, python,
  pip, npm run, or node.
• An ### Acceptance Criteria section with at least one checklist item (- [ ] ...).

Enhancements (enhancement label):

• A ### Desired Behavior section.
• An ### Acceptance Criteria section with at least one checklist item.

What is missing:

• The issue has neither the bug nor enhancement label, so its readiness criteria cannot be evaluated. Add the appropriate label.

Edit the issue to address the gaps and the ready-for-dev label will be
applied automatically.

This is an automated readiness check posted by a workflow.

[neubig]

A live false positive confirms this needs to be conservative: OpenHands/OpenHands#16223 was auto-approved even though the review explicitly reported a failing current-head Validate PR description gate; review 5279031279 has been dismissed.

Decisions for the acceptance criteria:

1. Treat every completed check run reported on the exact head with failure, cancelled, or timed_out as blocking unless its conclusion is neutral or skipped. Do not depend on ruleset or branch-protection visibility or a manually maintained check-name list; the reviewer token cannot reliably read rulesets, and repositories enforce merge-relevant policy through checks that may not appear in branch protection.
2. If any exact-head check is queued or in progress, exit without launching an agent and state that review is waiting on checks. A later explicit review request or the normal periodic/event process may retry; do not hold a worker slot polling.
3. Deduplicate deterministic remediation comments with a stable machine-readable marker that includes the head SHA and gate category. Update or replace the marked comment when the head changes instead of matching prose or author.
4. Apply this to both reviewer-request events and scheduled scans.

[neubig]

@all-hands-bot please implement this ready-for-dev issue and open a focused PR.

[all-hands-bot]

Implemented and opened a focused PR: #651

What changed

Eligibility gate before agent dispatch — skills/github-pr-reviewer/scripts/worker.py now evaluates the current head's check runs immediately before dispatcher.deliver(...), so it applies identically to the reviewer-request event path and the scheduled label scan.

• _classify_check_runs() ignores runs GitHub attributes to any other head SHA, so a stale failure cannot block the push that fixed it.
• A run whose status is not completed classifies as waiting. A completed run whose conclusion is not success, neutral, or skipped classifies as blocked. An unrecognized conclusion fails closed rather than approving silently, and a pending check can never produce a false approval.
• A non-green head logs a review-blocked / review-waiting disposition and continues before any conversation is created, without holding a worker slot to poll. An existing completed review still short-circuits first, so the gate never second-guesses a finished verdict.
• The trigger is not consumed. Scheduled mode leaves the label in place and the next scan retries; event mode records no delivery, so a later explicit all-hands-bot request starts the normal review once the head is green.

One explanation, deduplicated — _gate_comment() upserts its own comment through a stable hidden marker carrying the head SHA and gate category (<!-- openhands-review-gate:{blocked|waiting}:{sha} -->). A later run for a different head updates or replaces the marked comment instead of duplicating it, and an already-present equivalent repository-workflow remediation comment (identified by its no AI was used to generate this comment disclosure) is not duplicated.

Shared transport — skills/github/scripts/github_client.py gains GitHubRepository.check_runs(sha) for /repos/{owner}/{repo}/commits/{sha}/check-runs. That endpoint answers with an object rather than a list, so it paginates manually. Check-run reads use only the token the reviewer already has: no branch-protection or ruleset access, and no configured list of check names.

Docs and provenance — SKILL.md and README.md document the gate, the blocked/waiting outcomes, and the retry contract. manifest.json bumps the bundle and entry version 1.4.0 -> 1.5.0, with the matching fixture version; automations/bundle-index.js and skills/index.js were regenerated with npm run build.

Tests

tests/test_github_reviewer_delivery.py, driving the real shipped worker.py entrypoint through the catalog bundle helper:

• a failing exact-head check creates zero conversations and one marked explanation;
• a green head (success/skipped/neutral) creates exactly one;
• a pending check creates zero and reports waiting;
• a failure recorded on an obsolete head does not block the current head;
• an unrecognized conclusion blocks rather than approves;
• the gate comment is not duplicated for the same head, is updated when the head moves, and defers to an equivalent workflow comment;
• the event path respects the gate, and a waiting stop does not consume a later green request.

$ uv run pytest -q tests/test_github_reviewer_delivery.py tests/test_github_automation_foundation.py
all pass

$ uv run pytest -q
995 passed, 23 skipped

$ uv run python scripts/sync_extensions.py --check
clean (only the pre-existing non-blocking issue-duplicate-checker coverage warning)

$ npm run build
no drift

I also read the reported head through the new helper with the reviewer's own token, confirming the API surface and that the failing run is attributed to the exact head:

count 17
Validate PR description completed failure c49ff9c4350e

All checks on the PR are green. No change to the review prompt, verdict parsing, maintainer_handoff.py, profile/secret selection, or the Actions-based plugins/pr-review.

Generated by OpenHands AI on behalf of the user.

[neubig]

@all-hands-bot Please update the existing implementation in #651; do not open another PR. Its check-run-only gate misses Actions workflows that fail before creating jobs. #426 at head 41ffb9d44ff187953034e32fe096607f77e506dd had three failed pull_request workflow runs (Tests, Check Extensions, Deprecation deadlines), each with zero jobs, while the commit check-run rollup was green. The reviewer launched anyway. #459 reproduced the same gap.

Please have the pre-dispatch gate account for current-head workflow-run failures and pending runs as well as check runs, ignore obsolete heads, and add a focused regression test. Keep the existing gate and comment machinery. I posted the full reproduction on #651: #651 (comment).