Skip to content

Review CI-passing PRs without manual requests #658

Description

@neubig

Problem

The scheduled GitHub PR reviewer in #654 retries outstanding all-hands-bot requests, and #656 bounds how many new conversations one scan may launch. Once those requests are exhausted, the automation stops making progress even while open, non-draft PRs with passing CI still lack a review on their current head. A one-off batch of review requests would not cover later PRs.

Desired behavior

The existing scheduled reviewer continuously discovers eligible CI-passing PRs in its configured repositories and reviews them at a bounded pace. Explicit all-hands-bot requests continue to work. Keep this in the Extensions reviewer automation, reusing its scan, CI gate, keyed conversation delivery, native review, and maintainer handoff; do not add a separate host daemon or another automation.

Acceptance criteria

  • A scheduled scan considers every open, non-draft PR without a current-head all-hands-bot review, even if nobody requested the bot or applied a trigger label.
  • Before starting an agent, the scan requires the current head's applicable GitHub check runs and Actions workflow runs to be complete and non-failing. A failed or pending workflow with zero jobs is not mistaken for green. Explicit requests retain their existing gate behavior.
  • Unrequested PRs use a stable repository/PR/head delivery key so repeated scans create neither duplicate conversations nor duplicate native reviews. A new head is eligible again.
  • Use Bound scheduled PR review intake across OSS repositories #656's per-scan new-conversation limit across all configured repositories. Order candidates deterministically; explicit requests take priority, then the oldest eligible unreviewed PRs. Ineligible PRs do not consume slots.
  • A completed review uses the existing verdict and maintainer-handoff path. Handle bot-authored PRs through the reviewer's supported non-approval verdict path so they are not silently skipped because GitHub ignores self-review requests.
  • Focused tests cover an unrequested green PR, a failed zero-job workflow, a pending workflow, deduplication across scans, the cap, and a changed head. Live evidence shows a scheduled scan starting a bounded number of reviews across at least two of the four OSS repositories.

Dependencies

Stack after #654 and #656, or incorporate only the minimal shared code needed from them. #651's exact-head gate must include the zero-job workflow-run case reported on that PR. Do not merge automatically.


OpenHands AI triage

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

Triage

This is bounded scheduling work in the existing github-pr-reviewer automation, so no product or design decision is outstanding. It belongs in skills/github-pr-reviewer/scripts/worker.py, reusing the machinery the stacked PRs already add: the scheduled scan (run() with no event payload), the AgentConversationDispatcher.deliver(subject, delivery, prompt) keyed delivery that dedupes a {event-or-key}:{sha} delivery per subject, _finish_completed_review, the exact-head check gate (_classify_check_runs / _latest_check_runs), and run_repositories. No second automation record, host daemon, queue, or runtime layer is added.

Concrete inferred decisions, drawn from the adjacent code rather than invented:

  • Eligibility is "open, non-draft, and no completed native review by the reviewer account on the current head." The completion handler in worker.py already reads GET /pulls/{n}/reviews and filters by commit_id == head_sha and reviewer login, so the negative of that set is the candidate filter. The trigger is not a label or an outstanding request; it is the absence of a current-head review, so an unrequested PR is picked up without any label.
  • The delivery key is the existing deliver(subject="{repo_id}:pr:{n}", delivery="{key}:{sha}") mechanism. For an unrequested PR there is no reviewer-request event to key on (that is fix(review): resume requested reviews on a scheduled check scan #654's path for outstanding requests), so the natural stable key is the repository/PR identity plus the head SHA. fix(review): resume requested reviews on a scheduled check scan #654's keyed path and the explicit-request event path are unchanged.
  • Ordering and the cap follow Bound scheduled PR review intake across OSS repositories #656, whose merged behavior (per the stacked negotiation on that PR) is a configurable max_new_per_run defaulting to 2, keyed as max_new_per_run, shared across run_repositories' per-repository loop. Explicit all-hands-bot requests keep priority, then eligible unrequested PRs oldest-first; candidates whose gate is blocked/waiting post their disposition and consume no slot. Configure the maximum through the worker's rendered config, following the max_new_per_run key that github-issue-to-pr, gitlab-issue-to-mr, and github-agents-md-maintainer already use.
  • The "I review all current-head CI" rule is the existing check gate extended to Actions workflow runs. The precedent for the read is skills/github-delivery-watchdog/scripts/worker.py, which pages GET /actions/runs?head_sha={sha}, ignores runs whose head_sha differs, groups by (workflow_id, event, head_branch), takes the latest run_number per group, and requires every remaining run to be status == "completed" and conclusion == "success". This matches fix(review): gate agent launch on exact-head checks #651's stated goal on that PR (a head whose commit check-run rollup can even read SUCCESS while a pull_request workflow run failed with zero jobs) and the fix(review): gate agent launch on exact-head checks #651 comment requesting workflow-run coverage with the same blocked/waiting behavior, obsolete-head filtering, and no duplicated gate machinery. The gate stays single-pass: check-run and workflow-run findings combine into one disposition and one managed gate comment.

Stacking: base is #654 (which is itself stacked on #651), and the cap comes from #656. #651, #654, and #656 are all open and unmerged, so this issue is blocked on them; the implementation should target the stacked head rather than main. No merge is automatic and no OSS deployment change belongs here.

Non-goals: no repository-specific policy, VM/memory tuning, or deployment control in OpenHands/extensions; no second automation record, host daemon, or runtime layer; no new webhook event types, check_run parsing in the host, or GitHub permission scopes; no change to the review prompt, verdict parsing, maintainer handoff, secret selection, or the explicit review_requested / trigger-label paths beyond the shared gate. The live canary depends on the OSS deployment accepting a scheduled dispatch; the PR should state the live evidence it actually obtained.

Acceptance Criteria

  • With no trigger label and no all-hands-bot request, a scheduled scan considers an open, non-draft PR whose current head has no completed native review by the configured reviewer account, and starts a review for it. Drafts and PRs already carrying a current-head reviewer review are skipped.
  • Before starting an agent, the scan requires the exact head's applicable GitHub check runs to be complete and non-failing, with the existing policy: obsolete-head runs ignored, only the latest run of each logical check counted, and unknown conclusions failing closed.
  • Before starting an agent, the scan also requires the exact head's Actions workflow runs to be complete and non-failing: a workflow run whose status is not completed, or whose conclusion is not a non-blocking value, prevents the launch even when the commit's check-run rollup reports success. A workflow with zero jobs that failed or is pending is not treated as green.
  • A non-green head posts the existing managed gate comment (blocked or waiting) and starts no conversation; the deterministic comment is upserted, not duplicated, and the behavior is identical whether the finding came from a check run or a workflow run.
  • An explicit all-hands-bot request keeps its current trigger behavior and gate behavior, and is not regressed by the new unrequested-PR path.
  • Unrequested PRs use a stable repository/PR/head delivery key, so repeated scheduled scans create neither a second conversation nor a second native review for the same head. A PR whose head changes becomes eligible again for the new head and is reviewed exactly once under the new key.
  • A per-scan limit on new conversations applies across all configured repositories, defaulting to 2 and configurable via the max_new_per_run config key. The limit bounds conversations started, so deliveries that only deduplicate or report an already-running conversation do not consume it.
  • When more candidates are eligible than the limit allows, explicit requests are ordered before unrequested candidates, and unrequested candidates are ordered by PR age (oldest first) deterministically with stable tie-breakers; candidates whose gate is blocked or waiting consume no slot and do not prevent later eligible candidates from being considered in the same run.
  • When the limit is reached mid-scan, the run still completes its non-dispatching work for the remaining candidates (gate evaluation and gate comments, completed-review reconciliation, maintainer handoff) and does not abort; a per-PR or per-repository error is reported without aborting the other repositories.
  • A completed review uses the existing verdict parsing and maintainer-handoff path unchanged. A PR authored by the configured bot account is reviewed through the reviewer's supported non-approval verdict path (a COMMENT-event review that keeps the approved verdict) rather than being skipped because GitHub ignores a self-review request.
  • Focused automated tests driving the shipped reviewer entrypoint through the existing catalog-bundle test helper cover: an unrequested green PR that starts a review; a failed zero-job workflow run that blocks; a pending workflow run that waits; no duplicate conversation or review across repeated scans; the per-run cap limiting conversations across repositories; and a changed head becoming eligible again.
  • The reviewer SKILL.md and README.md document the unrequested-PR scan, the check-run plus workflow-run gate, the delivery key, and the per-run maximum with its default; the catalog entry's bundle and entry versions are bumped and the generated catalogs are regenerated so python scripts/sync_extensions.py --check and npm run build report no drift.
  • No new secret, token scope, or GitHub permission is required beyond what the current reviewer uses, and no secret value appears in logs, comments, or the PR.
  • Live evidence in the PR's AGENT section shows a scheduled scan starting a bounded number of reviews across at least two of the four OSS repositories, with links to the reviewed pull requests and their conversations.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority:normalNormal priorityready-for-devScoped for contribution; managed by repository readiness checks.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions