fix(reviewer): honor explicit requests on draft PRs

[neubig]

HUMAN:

Tested in the canvas already, and it works.

AGENT:

• A human has tested these changes.

---

Why

A draft with an explicit all-hands-bot request was silently excluded from scheduled review recovery. For example, SDK #5350 has green CI and an outstanding review request, but hourly scans never selected it.

Summary

Remove the draft check from the existing outstanding-request predicate. This makes explicitly requested drafts eligible without changing the unsolicited-draft filter, CI policy, authorization, request deduplication, or scan limits. Update the setup instructions/catalog and regenerate their derived assets.

Fixes #691.

How to Test

Validate the changed predicate and reviewer delivery:

• uv run pytest -q tests/test_github_reviewer_delivery.py tests/test_github_reviewer_main.py
• npm run build:automations

Evidence:

158 tests passed across reviewer delivery, reviewer main, and shared GitHub automation tests. Coverage includes event and scheduled draft requests, waiting for CI, repeated-scan delivery identity, and excluding drafts requested from someone else or never requested.

Deployed only the changed request predicate to the current OSS reviewer bundle, preserving the rest of its implementation/configuration. The downloaded live bundle now accepts a requested draft and still rejects an unrequested draft or one requested from another reviewer. The scheduled scan completed successfully.

The Automation Git sync endpoint persisted exactly this method change in OpenHands/oss-agent-canvas commit a99a271, on the existing factory/reviewer-continuous-canary sync branch (PR #11). Live scheduled scans 064d5d11-8460-4955-86a1-6243f71f4d16 and 44a60b06-f5d4-4e7a-a0a3-fdf96690ccd7 completed successfully on 2026-09-27. They selected explicitly requested drafts OpenHands #17211 and SDK #5209, then SDK #5350 while all three remained draft. The next scan reported the first two as in_progress, reusing their conversations.

SDK #5350 was admitted at exact head f9b91576de526079fa982a390c7945335c7cda8b with disposition created: live review conversation. This demonstrates admission and execution of the requested draft, not completion or approval of its code review. The existing two-new-conversations-per-scan setting was preserved.

This PR description was updated by an AI agent (OpenHands) on behalf of Graham Neubig.

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Summary

The change removes the pr.get("draft") short-circuit from _outstanding_review_request() in skills/github-pr-reviewer/scripts/worker.py. That predicate is the only draft gate on the explicit-request path, so the fix is minimal and targeted: an open PR that still holds a live all-hands-bot request is now eligible for scheduled recovery even while it is a draft. I traced every call site to confirm the scope is exactly what the PR claims:

• _explicit_candidate() (worker.py:823) now classifies a requested draft as explicit, so it is admitted to the rotating window and never windowed.
• run() (worker.py:880) recomputes requested from the live PR payload; a requested draft falls into the elif requested: branch and is keyed by its own review_requested event, so deduplication identity ({event_id}:{head}) is unchanged.
• The unrequested path (worker.py:900-906) still short-circuits if pr.get("draft"): continue, so unsolicited drafts remain excluded.
• _rotating_window() keeps its pre-classification draft filter for drafts that are neither labeled nor requested.

The base behavior for non-draft PRs, the CI gate, authorization (credential must authenticate as trigger_reviewer), the per-scan intake bound, and SCAN_WINDOW rotation are unaffected.

Verification performed on this head (d0011f0)

• Current head still matches d0011f0799927ed1de8002999fdb9a0f090f35eb; PR is open with label type: fix; linked issue #691 is open with ready-for-dev + priority: medium. No prior reviews or review threads exist on the PR.
• GitHub check runs for the head are all success/skipped (pr-title, test, check, check-pr-artifacts, validate-claude-code, sync-*).
• uv run pytest tests/ -q -> 1064 passed, 23 skipped; tests/test_github_reviewer_delivery.py -> 89 passed, including the new parametrized draft cases for event mode, scheduled recovery, and repeated-scan dedup.
• python scripts/sync_extensions.py --check passes, and re-running npm run build:skills / npm run build:automations produced no diff, so skills/index.js, automations/bundle-index.js, and automations/catalog-index.js are the correct regenerated output. The embedded bundle worker.py is byte-identical to skills/github-pr-reviewer/scripts/worker.py.

Non-blocking observation

The prose update is incomplete relative to the behavior change. skills/github-pr-reviewer/SKILL.md:116 still reads: "Draft, scope, exact-head, and delivery-deduplication safeguards still apply" for an explicit all-hands-bot request. After this change, draft is no longer a filter on the explicit-request path in either event mode or scheduled scans, so that sentence now implies the opposite of the new line 61 ("every open PR, including drafts, ... outstanding request") and could mislead an agent reading the skill. Consider removing "Draft" from that list. The parallel non-draft wording for the unrequested window (README, the scheduled-scan prompt string at worker.py:262, and the manifest description/help) remains accurate and needs no change.

Verdict

No blocking or material correctness, security, or design issue found. The fix is scoped, consistent with the repository's cross-repository and generated-asset conventions, and covered by regression tests.

✅ APPROVED

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Summary

The change removes the pr.get("draft") short-circuit from _outstanding_review_request() in skills/github-pr-reviewer/scripts/worker.py. That predicate is the only draft gate on the explicit-request path, so the fix is minimal and targeted: an open PR that still holds a live all-hands-bot request is now eligible for scheduled recovery even while it is a draft. I traced every call site to confirm the scope is exactly what the PR claims:

• _explicit_candidate() (worker.py:823) now classifies a requested draft as explicit, so it is admitted to the rotating window and never windowed.
• run() (worker.py:880) recomputes requested from the live PR payload; a requested draft falls into the elif requested: branch and is keyed by its own review_requested event, so the deduplication identity ({event_id}:{head}) is unchanged.
• The unrequested path (worker.py:900-906) still short-circuits if pr.get("draft"): continue, so unsolicited drafts remain excluded.
• _rotating_window() keeps its pre-classification draft filter for drafts that are neither labeled nor requested.

Base behavior for non-draft PRs, the CI gate, authorization (credential must authenticate as trigger_reviewer), the per-scan intake bound, and SCAN_WINDOW rotation are unaffected.

Verification performed on this head (d0011f0)

• Current head still matches d0011f0799927ed1de8002999fdb9a0f090f35eb; PR is open with label type: fix; linked issue #691 is open with ready-for-dev + priority: medium.
• All GitHub check runs for the head are success/skipped (test, check, check-pr-artifacts, validate-claude-code, sync-extensions, sync-sdk-skill, pr-title).
• uv run pytest tests/test_github_reviewer_delivery.py -q -> 89 passed, including the parametrized draft cases for event mode, scheduled recovery, and repeated-scan dedup.

Non-blocking observation

The prose update is incomplete relative to the behavior change. skills/github-pr-reviewer/SKILL.md:116 still reads: "Draft, scope, exact-head, and delivery-deduplication safeguards still apply" for an explicit all-hands-bot request. After this change, draft is no longer a filter on the explicit-request path in either event mode or scheduled scans, so that sentence now contradicts the new line 61 ("every open PR, including drafts, ... outstanding request") and could mislead an agent reading the skill. Consider removing "Draft" from that list. The parallel non-draft wording for the unrequested window (README, the scheduled-scan prompt string at worker.py:262, and the manifest description/help) remains accurate and needs no change.

Verdict

No blocking or material correctness, security, or design issue found. The fix is scoped, consistent with the repository's cross-repository and generated-asset conventions, and covered by regression tests.

✅ APPROVED

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Summary

The code change is unchanged from the previously reviewed head and remains correct. It removes the pr.get("draft") short-circuit from _outstanding_review_request() in skills/github-pr-reviewer/scripts/worker.py. That predicate is the only draft gate on the explicit-request path, so an open PR holding a live all-hands-bot request is now eligible for scheduled recovery while it is a draft:

• _explicit_candidate() (worker.py:823) classifies a requested draft as explicit, so it is admitted to the rotating window and never windowed.
• run() (worker.py:880) recomputes requested from the live PR payload; a requested draft takes the elif requested: branch and is keyed by its own review_requested event, so the deduplication identity ({event_id}:{head}) is unchanged.
• The unrequested path (worker.py:900-906) still short-circuits if pr.get("draft"): continue, so unsolicited drafts remain excluded.

Base behavior for non-draft PRs, the CI gate, authorization, the per-scan intake bound, and SCAN_WINDOW rotation are unaffected. Focused tests pass on this head: uv run pytest tests/test_github_reviewer_delivery.py -q -> 89 passed, including the parametrized draft cases for event mode, scheduled recovery, and repeated-scan dedup. test, check, validate-claude-code, sync-extensions, and sync-sdk-skill all succeed for the head.

Blocking finding: the Validate PR description gate now fails

This PR has been marked ready for review (it is no longer a draft). That transition re-triggered .github/workflows/pr-description-check.yml (pull_request_target: ready_for_review), whose job condition github.event.pull_request.draft == false now evaluates true. The job ran at 2026-09-28T15:02:23Z against this exact head and failed:

##[error]Keep the `## Summary` section from the PR template.
##[error]Keep the `## How to Test` section from the PR template.
PR description validation failed with 2 error(s).

I reproduced it locally against the current PR body:

python .github/scripts/check_pr_description.py --body-file /tmp/pr_body.md
::error::Keep the `## Summary` section from the PR template.
::error::Keep the `## How to Test` section from the PR template.

The current body uses ## Why, ## Changes, and ## Validation. .github/scripts/check_pr_description.py requires the ## Why, ## Summary, and ## How to Test sections from .github/pull_request_template.md to be present and non-empty (the ## Why and linked-issue checks pass; the linked issue #691 carries ready-for-dev). Why this matters: the repository treats the PR-description check as a gate, and the current head is reported as FAILURE in the status rollup (mergeStateStatus: UNSTABLE). It was only masked earlier because the check is skipped while the PR is a draft. Before this PR can present a clean state, the author should add the missing ## Summary and ## How to Test sections to the description.

This is a PR-description issue, so it cannot be attached to a changed line and is reported here.

Verdict

The code change itself has no material correctness, security, or design issue. However, the current head does not present a clean state: the repository's Validate PR description gate fails because the now-ready-for-review PR body is missing the required ## Summary and ## How to Test sections. That is an actionable fix before merge.

🔄 CHANGES REQUESTED

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Summary

The code change on this head is correct and unchanged. It removes the pr.get("draft") short-circuit from _outstanding_review_request() in skills/github-pr-reviewer/scripts/worker.py. That predicate is the only draft gate on the explicit-request path, so an open PR holding a live all-hands-bot request is now eligible for scheduled recovery while it remains a draft:

• _explicit_candidate() (worker.py:823) now classifies a requested draft as explicit, so it enters the rotating window and is never windowed.
• run() (worker.py:880) recomputes requested from the live PR payload; a requested draft takes the elif requested: branch and is keyed by its own review_requested event, so the deduplication identity ({event_id}:{head}) is unchanged.
• The unrequested path (worker.py:900-906) still short-circuits if pr.get("draft"): continue, so unsolicited drafts remain excluded.

Base behavior for non-draft PRs, the CI gate, authorization, the per-scan intake bound, and SCAN_WINDOW rotation are unaffected. Focused tests pass on this head: uv run pytest tests/test_github_reviewer_delivery.py -q -> 89 passed. The required status checks (test, sync-extensions, validate-claude-code) all succeed for the head.

Blocking finding: the Validate PR description gate is red

The PR is ready for review (not a draft), so .github/workflows/pr-description-check.yml runs its job (condition github.event.pull_request.draft == false). Its most recent run against this exact head (2026-09-28T15:02:23Z) failed:

##[error]Keep the `## Summary` section from the PR template.
##[error]Keep the `## How to Test` section from the PR template.
PR description validation failed with 2 error(s).

Reproduced locally against the current PR body:

python .github/scripts/check_pr_description.py --body-file /tmp/pr_body.md
::error::Keep the `## Summary` section from the PR template.
::error::Keep the `## How to Test` section from the PR template.

The current body uses ## Why, ## Changes, and ## Validation. .github/scripts/check_pr_description.py requires the ## Why, ## Summary, and ## How to Test sections from .github/pull_request_template.md to be present and non-empty (the ## Why and linked-issue checks pass; issue #691 carries ready-for-dev). This is why the head reports statusCheckRollup: FAILURE and mergeStateStatus: UNSTABLE. It was previously masked because the check is skipped while the PR is a draft. Before the PR can present a clean state, the description should add the missing ## Summary and ## How to Test sections.

This is a PR-description issue, so it cannot be attached to a changed line and is reported here.

Verdict

The code change itself has no material correctness, security, or design issue. The current head does not present a clean state, however: the repository's Validate PR description gate still fails because the ready-for-review PR body is missing the required ## Summary and ## How to Test sections. That is an actionable fix before merge.

🔄 CHANGES REQUESTED

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Summary

The code change is correct and unchanged from the previously reviewed head. It removes the pr.get("draft") short-circuit from _outstanding_review_request() in skills/github-pr-reviewer/scripts/worker.py. That predicate is the only draft gate on the explicit-request path, so an open PR that still holds a live all-hands-bot request is now eligible for scheduled recovery while it is a draft:

• _explicit_candidate() (worker.py:823) classifies a requested draft as explicit, so it enters the rotating window and is never windowed.
• run() (worker.py:880) recomputes requested from the live PR payload; a requested draft takes the elif requested: branch and is keyed by its own review_requested event, so the deduplication identity ({event_id}:{head}) is unchanged.
• The unrequested path (worker.py:900-906) still short-circuits if pr.get("draft"): continue, so unsolicited drafts remain excluded.

Base behavior for non-draft PRs, the CI gate, authorization, the per-scan intake bound, and SCAN_WINDOW rotation are unaffected.

Verification performed on this head (d0011f0)

• Current head still matches d0011f0799927ed1de8002999fdb9a0f090f35eb; PR is open and ready for review with label type: fix; linked issue #691 is open with ready-for-dev + priority:medium; the outstanding review request is still all-hands-bot.
• The three checks named in the main ruleset (id 11690949) - test, sync-extensions, validate-claude-code - all succeed on the exact head.
• uv run pytest tests/test_github_reviewer_delivery.py -q -> 89 passed, including the parametrized draft cases for event mode, scheduled recovery, and repeated-scan dedup for a requested draft, plus the renamed test that a draft requested from another reviewer stays excluded.
• uv run python scripts/sync_extensions.py --check exits 0 (only a pre-existing non-blocking coverage warning about plugins/issue-duplicate-checker). Re-running the sync/build produced no diff, and the embedded worker.py in automations/bundle-index.js is byte-identical to skills/github-pr-reviewer/scripts/worker.py, so the generated assets match the hand-authored source.

Blocking finding: the Validate PR description gate is red against this head

The PR is ready for review, so .github/workflows/pr-description-check.yml runs its job (condition github.event.pull_request.draft == false). Its most recent run against this exact head (2026-09-28T15:02:23Z, run 36440533075 / job 108989524923) failed:

##[error]Keep the `## Summary` section from the PR template.
##[error]Keep the `## How to Test` section from the PR template.
PR description validation failed with 2 error(s).

Reproduced locally against the current PR body:

python .github/scripts/check_pr_description.py --body-file /tmp/pr_body.md
::error::Keep the `## Summary` section from the PR template.
::error::Keep the `## How to Test` section from the PR template.

The current body uses ## Why, ## Changes, and ## Validation. .github/scripts/check_pr_description.py requires the ## Why, ## Summary, and ## How to Test sections from .github/pull_request_template.md to be present and non-empty (the ## Why and linked-issue checks pass; issue #691 carries ready-for-dev). This check was only masked earlier because it is skipped while the PR is a draft. It is not currently one of the three ruleset-required contexts, so it does not by itself make the head unmergeable, but the author should add the missing ## Summary and ## How to Test sections so the head presents a clean, expected state.

This is a PR-description issue, so it cannot be attached to a changed line and is reported here.

Verdict

The code change itself has no material correctness, security, or design issue, and it is scoped exactly as described with regression coverage. The current head does not present a clean state, however: the repository's Validate PR description gate fails because the ready-for-review PR body is missing the required ## Summary and ## How to Test sections. That is an actionable fix before merge.

🔄 CHANGES REQUESTED

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Summary

The code change on this head is correct. _outstanding_review_request() in skills/github-pr-reviewer/scripts/worker.py no longer short-circuits on pr.get("draft"), so an open PR that still holds a live all-hands-bot request is eligible for scheduled recovery while it is a draft:

• _explicit_candidate() now classifies a requested draft as explicit, so it enters the candidate set and is never windowed.
• run() recomputes requested from the live PR payload; a requested draft is keyed by its own review_requested event, so the deduplication identity ({event_id}:{head}) is unchanged.
• The unrequested path still short-circuits on pr.get("draft"), so unsolicited drafts remain excluded.

Base behavior for non-draft PRs, the CI gate, authorization, the per-scan intake bound, and rotation are unaffected. Full test suite on this head: uv run pytest tests/ -q -> 1087 passed, 23 skipped (focused delivery subset: 94 passed). The three ruleset-required contexts (test, sync-extensions, validate-claude-code) all succeed on the exact head.

Blocking finding 1: the branch conflicts with current main

GitHub reports mergeable: CONFLICTING, mergeStateStatus: DIRTY for head 2b81712. The head is a merge of main up to 41593ce (#699), but main has since advanced to 9c140e2 (#703, "scan backlog before applying launch quota"), which this branch does not contain. I reproduced the conflict in the workspace with git merge-tree --write-tree origin/main HEAD; it reports content conflicts in three files this PR edits:

• automations/bundle-index.js
• automations/catalog/github-pr-reviewer/manifest.json
• skills/index.js

The conflicting lines are the generated reviewer version and description: #703 bumped the reviewer catalog entry and bundle to 1.8.1 and rewrote its description/window wording, while this PR also edits those same lines from the 1.8.0 base. Until the branch is rebased/merged onto current main and the generated assets are regenerated, the PR cannot be merged. This matters to the merge decision because the conflicting generated assets are exactly the artifacts this PR claims to have regenerated; resolving requires re-running the repo's build/sync against current main, not just a textual conflict resolution.

Blocking finding 2: the Validate PR description gate is red

The PR is ready for review, so .github/workflows/pr-description-check.yml runs. Its run against this exact head (run 36576647183 / job 109433877521, 2026-09-29T13:39:06Z) failed with 3 errors:

##[error]The first visible line of the PR description must be `HUMAN:`.
##[error]Add a short human-written note (at least 20 characters) between `HUMAN:` and `AGENT:`.
##[error]Keep the `AGENT:` marker from the PR template.
PR description validation failed with 3 error(s).

Reproduced locally against the current PR body with python .github/scripts/check_pr_description.py --body-file -> the same 3 errors. Current main now requires the body to start with a HUMAN: line, a human note of at least 20 visible characters, and the AGENT: marker (see .github/pull_request_template.md and the three-gate docstring in .github/scripts/check_pr_description.py, which now lists human/agent authorship as gate 1). The current body starts with - [ ] A human has tested these changes. and contains no HUMAN:/AGENT: markers. The template-section errors from the prior head are now resolved (## Why, ## Summary, and ## How to Test are present), and the linked issue #691 carries ready-for-dev.

These are PR-body/merge-state issues, so they cannot be attached to a changed line and are reported here.

Verdict

The code change itself is scoped exactly as described and is covered by regression tests. The current head does not present a mergeable or clean state: it conflicts with current main in three generated assets (including the 1.8.1 version bump from #703), and the repository's Validate PR description gate fails because the body lacks the newly required HUMAN:/AGENT: markers. Rebase onto current main, regenerate the assets, and update the description format before merge.

🔄 CHANGES REQUESTED

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Summary

This is a fresh review of the new head 1a8d551 (a Merge main into fix/review-explicit-draft-requests commit). The code change is unchanged from the earlier heads and remains correct. _outstanding_review_request() in skills/github-pr-reviewer/scripts/worker.py no longer short-circuits on pr.get("draft"), so an open PR that still holds a live all-hands-bot request is eligible for scheduled recovery while it is a draft:

• _scan_candidates() (worker.py:823-838) keeps its pr.get("draft") and label not in labels and not self._outstanding_review_request(pr) filter, so a draft with an explicit request now passes the pre-filter and enters the candidate set.
• run() (worker.py:864) recomputes requested from the live PR payload; a requested draft takes the elif requested: branch and is keyed by its own review_requested event, so the deduplication identity ({event_id}:{head}) is unchanged.
• The unrequested path (worker.py:888) still short-circuits if pr.get("draft"): continue, so unsolicited drafts remain excluded.

The merge integrated the change correctly on top of the newer main (#703/#699) code: the draft predicate now lives in _scan_candidates() rather than the removed _rotating_window(), and the explicit-request branch is preserved. Base behavior for non-draft PRs, the CI gate, authorization, the shared per-scan intake bound, and the full-backlog classification introduced by #703 are unaffected.

Verification performed on this head (1a8d551)

• Head still matches 1a8d55100683e3fe98b3962b9b2da73a0935f3d6; PR is open and ready for review with label type: fix; linked issue #691 is open with ready-for-dev + priority:medium; the outstanding review request is still all-hands-bot.
• The previous mergeable: CONFLICTING state is gone: GitHub now reports mergeable: true for this head, so the earlier conflict with main in automations/bundle-index.js, automations/catalog/github-pr-reviewer/manifest.json, and skills/index.js has been resolved by the merge commit.
• The three ruleset-required contexts (test, sync-extensions, validate-claude-code) all succeed on the exact head.
• uv run pytest -q tests/test_github_reviewer_delivery.py -> 92 passed, including the parametrized draft cases for event mode, scheduled recovery, and repeated-scan dedup for a requested draft, plus the renamed test that a draft requested from another reviewer stays excluded. uv run pytest -q skills/github-pr-reviewer/tests/test_main.py -> 57 passed.
• uv run python scripts/sync_extensions.py --check exits 0, and npm run build:automations leaves git status clean, so the generated assets match the hand-authored source. The embedded worker.py in automations/bundle-index.js is byte-identical to skills/github-pr-reviewer/scripts/worker.py.

Blocking finding: the Validate PR description gate is red against this head

The PR is ready for review, so .github/workflows/pr-description-check.yml runs. Its run against this exact head (run 36602212490 / job 109522144831, 2026-09-29T17:03:28Z) failed with 3 errors:

##[error]The first visible line of the PR description must be `HUMAN:`.
##[error]Add a short human-written note (at least 20 characters) between `HUMAN:` and `AGENT:`.
##[error]Keep the `AGENT:` marker from the PR template.
PR description validation failed with 3 error(s).

Reproduced locally against the current PR body with python .github/scripts/check_pr_description.py --body-file -> the same 3 errors. .github/pull_request_template.md and the three-gate docstring in .github/scripts/check_pr_description.py now require the first visible line to be HUMAN:, a human note of at least 20 visible characters before AGENT:, and the AGENT: marker. The current body starts with - [ ] A human has tested these changes. and contains no HUMAN:/AGENT: markers. The template-section errors from the prior head are now resolved (## Why, ## Summary, and ## How to Test are present) and the linked issue #691 carries ready-for-dev.

This is a PR-description issue, so it cannot be attached to a changed line and is reported here. It is not one of the three ruleset-required contexts, so it does not by itself make the head unmergeable, but the author should reformat the description with the HUMAN:/AGENT: markers so the head presents a clean, expected state.

Verdict

The code change itself is scoped exactly as described and is covered by regression tests; the earlier merge conflict with main is resolved. The one outstanding actionable item is the repository's Validate PR description gate, which fails because the body lacks the newly required HUMAN:/AGENT: markers. Update the description format before merge.

🔄 CHANGES REQUESTED

[openhands-release-bot]

🚀 Released in v0.26.0.