fix(pr-review): refresh GitHub state when resuming keyed reviews - #649
Merged
Merged
Conversation
A keyed review conversation is resumed for an explicit re-review, but the review prompt only required re-reading the PR head and current-head Actions results. The remaining mutable GitHub state - PR body, review comments and threads, review requests, and linked issue bodies and labels - was never required to be re-read, so a resumed agent could report an earlier turn's observation as current (a linked issue that gained ready-for-dev and changed priority between two same-head review requests). Add a CURRENT STATE workflow step that runs before the scope gate and before any verdict, requiring the reviewer to re-establish the exact head, PR body, review threads, review requests, current-head check results, and every linked issue's current body and labels, while retaining earlier analysis and repository guidance as useful background. The event-mode acceptance block in the worker likewise requires confirming those surfaces before reporting. Keyed conversation reuse, delivery dedup, and the no-new-conversation contract are unchanged. Tests reproduce a completed first review, mutate linked-issue and head check/review state, resume the same conversation, and assert the second review uses the refreshed state. Docs and the generated bundle stay synchronized; bundle version bumped 1.4.0 -> 1.5.0. Closes #648 Co-authored-by: openhands <openhands@all-hands.dev>
7 tasks
Member
|
@all-hands-bot The PR description validator is failing on the current head. Please inspect the validator output and update only the PR description to satisfy the repository template while preserving the substantive summary, verification, and explicit human-authorship/review disclosure. Do not change code unless a current-head check identifies a code issue. |
Contributor
Author
|
Inspected the failing check on the current head (
I updated only the PR description — no code changes. The rewrite:
I ran the validator script locally against the new body before updating: No current-head code issue was identified — the other checks ( Generated by OpenHands AI on behalf of the user. |
1 task
all-hands-bot
pushed a commit
that referenced
this pull request
Sep 22, 2026
GitHub lists every run for a commit, so a check re-run after a fix kept contributing its superseded failure forever. Group check runs by name and reporting app identity, pick the latest by start time and then run ID, and classify only that run. A later queued or in-progress re-run supersedes an earlier success, so the head waits instead of being reviewed. Adds regression tests for the live #647/#649 repeated Validate PR description runs, the start-time tie broken by run ID, a newer pending re-run, and same-name checks from different apps. Co-authored-by: openhands <openhands@all-hands.dev>
all-hands-bot
pushed a commit
that referenced
this pull request
Sep 22, 2026
GitHub lists every run for a commit, so a check re-run after a fix kept contributing its superseded failure forever. Group check runs by name and reporting app identity, pick the latest by start time and then run ID, and classify only that run. A later queued or in-progress re-run supersedes an earlier success, so the head waits instead of being reviewed. Adds regression tests for the live #647/#649 repeated Validate PR description runs, the start-time tie broken by run ID, a newer pending re-run, and same-name checks from different apps. Co-authored-by: openhands <openhands@all-hands.dev>
neubig
approved these changes
Sep 23, 2026
neubig
left a comment
neubig
left a comment
Member
There was a problem hiding this comment.
The reproduced stale-state failure is valid, and the fix preserves keyed conversation reuse while explicitly re-establishing every mutable GitHub fact before scope or verdict decisions. Prompt-contract tests, dispatcher reuse coverage, synchronized bundles, and current CI all support the change.
AI disclosure: This review was submitted by OpenHands on behalf of @neubig.
Contributor
|
🚀 Released in v0.24.0. |
neubig
pushed a commit
that referenced
this pull request
Sep 25, 2026
* fix(review): reject failed required checks before launching an agent The reviewer dispatched an agent conversation for every eligible trigger without reading the current head's check state, so a head with an already failing required check still spent an LLM slot (OpenHands/OpenHands#17544 created conversation f46343fd despite a failing Validate PR description). Add a deterministic head-eligibility gate in front of dispatcher.deliver: - GitHubRepository.check_runs(sha) reads the check runs the reviewer's own token can already see, with no ruleset or branch-protection access. - A completed run on the exact head blocks unless its conclusion is success, neutral, or skipped, so an unknown conclusion fails closed. - A queued or in_progress run exits as waiting on checks without holding a slot to poll, and never approves. - Runs attributed to another head are ignored. - The trigger is not consumed: the next scan or explicit review request starts the review once the head is green. - One explanation is left per head, upserted through a marker carrying the head SHA and gate category, and an equivalent repository workflow remediation comment is not duplicated. Bump the bundle version to 1.5.0 and regenerate the catalog artifacts. Co-authored-by: openhands <openhands@all-hands.dev> * fix(review): trust only owned gate comments and matching dedupe Only treat a marked comment as managed when the configured reviewer account authored it, so a marker placed by a PR author can neither suppress the gate explanation nor be PATCHed. Narrow the workflow dedupe to a disclosure comment that names the same current-head checks. Rename the blocked heading to current-head checks. Add narrow regression tests for the ownership and dedupe cases. * fix(review): gate on the latest run of each exact-head check GitHub lists every run for a commit, so a check re-run after a fix kept contributing its superseded failure forever. Group check runs by name and reporting app identity, pick the latest by start time and then run ID, and classify only that run. A later queued or in-progress re-run supersedes an earlier success, so the head waits instead of being reviewed. Adds regression tests for the live #647/#649 repeated Validate PR description runs, the start-time tie broken by run ID, a newer pending re-run, and same-name checks from different apps. Co-authored-by: openhands <openhands@all-hands.dev> * fix(review): order a queued check run without a start time by its run ID A queued or requested check run can exist before GitHub sets started_at, which the API reports as null. Ordering used the empty string for a missing start time, so an earlier completed success with a timestamp outranked the newer queued rerun and the reviewer could launch while the rerun was still pending. Fall back to the run ID, which increases monotonically, whenever a start time is absent. Co-authored-by: openhands <openhands@all-hands.dev> * fix(review): order exact-head check runs by run ID The `~{run_id}` fallback sorted a run without a `started_at` past every real timestamp, so an older queued/cancelled run could outrank a newer successful run on the same check and keep a head blocked or waiting. The check-run ID is the monotonic creation sequence, so make it the primary order key and use the start time only as a tie-break. Adds the reverse regression alongside the existing null-start case. * fix(review): gate on current-head workflow runs too The head-eligibility gate read only check runs, so a workflow that failed before creating any check run left the commit's check-run rollup green and the reviewer launched for red CI. On #426 head 41ffb9d, `Tests`, `Check Extensions`, and `Deprecation deadlines` failed with zero jobs, so only the green `pr-title` checks were visible; the reviewer spent an agent slot before reporting the failures, and `gh pr checks` omitted the zero-job runs as well. Read the current head's Actions workflow runs as well as its check runs: - GitHubRepository.workflow_runs(sha) reads `/actions/runs?head_sha=<sha>`, which answers with an object and so paginates manually like check_runs. - A workflow run whose check suite already reported check runs is left to those runs, so a workflow is never counted twice. Only a run whose suite reported no check runs - a workflow-level failure, or a `pull_request` run whose jobs never started - is added to the gate. - Workflow runs get the same blocked/waiting behavior and the same current-head filter as check runs, and the latest run of each workflow (name + workflow ID, by run ID then start time) supersedes its superseded attempts. - The two grouping paths share one `_latest_by_group` helper and the one existing classifier and comment machinery. Co-authored-by: openhands <openhands@all-hands.dev> * fix(review): gate scheduled discovery on required checks only The live deployment exposed two over-broad behaviors in the exact-head eligibility gate. OpenHands/OpenHands#17200 passes `test-and-build (ubuntu)`, its only GitHub-required check, while the optional `release ready` workflow has a zero-job `startup_failure`. Classifying every check and workflow run treated that optional failure as blocking, so scheduled discovery paused a mergeable head. An explicit `all-hands-bot` review request is the intake-policy exception: the caller asked for that head by name, so it must dispatch even when required CI is red or pending. - GitHubRepository.required_check_contexts(number) adds one GraphQL query that returns the head's status check rollup and keeps only the contexts isRequired(pullRequestNumber:) marks required. That field is merge-policy source of truth and is pull-request-scoped, so it is correct for a stacked PR whose symbolic base branch has no rules of its own. - Scheduled classification uses only those required contexts: a required check run by name, a required commit status by context. An optional failure cannot block. A required context with no current-head run is expected, so it waits. - When the required signal cannot be read or is empty the gate falls back to every current-head check run and workflow run, so a red head still blocks. This keeps the #426 zero-job shape blocked: its required checks never started, isRequired returns no node, and the empty set falls back. - _gate_head(pr, requested=True) returns green without reading checks and without a gate comment, and run() passes requested=event_mode. Exact-head filtering, latest-run supersession, fail-closed conclusions, the comment marker and its ownership/dedupe rules, and the conversation dispatch path are unchanged. Co-authored-by: openhands <openhands@all-hands.dev> --------- Co-authored-by: openhands <openhands@all-hands.dev>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Fixes #648. A keyed review conversation is resumed for an explicit re-review, but
the review prompt only required the reviewer to re-read the PR head and inspect
current-head Actions results. The remaining mutable GitHub state was never
required to be re-read, so a resumed agent could report an earlier turn's
observation as current. The reported case:
OpenHands/software-agent-sdk#5039was reviewed at 14:05 UTC; its linked issue
#5038gainedready-for-devand achanged priority at 14:11 UTC; an explicit re-review at 14:14 UTC at the same
head reposted the earlier verdict claiming the issue was still
priority:mediumand not ready.
Root cause. Conversation reuse is intentional:
AgentConversationDispatcher.deliver()resumes the stable per-PR conversation and, for a new delivery, sends exactly one
fresh turn. The defect was in the prompt that turn carries.
_build_review_prompt()in
skills/github-pr-reviewer/scripts/main.pyrequired only a head re-read andcurrent-head Actions inspection, leaving PR body, review comments/threads,
review requests, and linked-issue bodies and labels unrefreshed.
Summary
skills/github-pr-reviewer/scripts/main.py— add workflow step 2, CURRENTSTATE, which runs before the scope gate and before any verdict. It requires
re-fetching the exact head (and whether it still matches), the current PR title
and body, review comments and threads, review requests, current-head Actions
results, and the current body and labels of every linked issue the PR
references. It states explicitly that earlier analysis and already-read
repository guidance remain useful background, but every mutable fact must be
re-established, and an earlier finding, verdict, or label/priority claim the
current state does not support must not be repeated. Remaining steps are
renumbered and the step-8 fallback reference updated. Because this is the
shared prompt, both scheduled label mode and event mode inherit it.
skills/github-pr-reviewer/scripts/worker.py— the event-mode acceptanceblock now also requires confirming the PR's current body, labels, review
threads, and head check results, and re-reading every linked issue's current
body and labels, immediately before reporting.
skills/github-pr-reviewer/SKILL.md,skills/github-pr-reviewer/README.md—document that an explicit re-review refreshes mutable GitHub state rather than
trusting earlier observations, while repository analysis already done is
retained.
no-new-conversation-for-an-existing-subject contract are untouched.
Issue Number
Fixes #648
How to Test
uv run pytest skills/github-pr-reviewer/tests -q— expect 57 passed.uv run pytest -q— expect 984 passed, 23 skipped.uv run python scripts/sync_extensions.py --check— expect clean. Thepre-existing non-blocking
issue-duplicate-checkercoverage warning isunrelated to this change.
npm run build— expect regeneratedautomations/bundle-index.jsandskills/index.jswith no drift.test_same_head_re_review_resumes_keyed_conversation_with_refreshed_statefromtests/test_github_reviewer_delivery.py; it fails without the fix.Added regression coverage:
tests/test_github_reviewer_delivery.py—test_same_head_re_review_resumes_keyed_conversation_with_refreshed_statereproduces the reported scenario against the real dispatcher (SDK/KV
boundaries stubbed): a first review completes, the linked issue gains
readiness and the head's checks/reviews move, then an explicit re-review at the
same head. It asserts the second delivery attaches the same keyed conversation
id, sends exactly one new turn, and that the resumed prompt carries the
refreshed PR state plus the requirement to re-read every mutable surface. This
test fails on the unpatched code.
skills/github-pr-reviewer/tests/test_main.py—test_prompt_requires_refreshing_mutable_github_statepins each surface,including
ready-for-dev;test_prompt_forbids_repeating_an_earlier_observationpins the no-stale-claims wording; and
test_prompt_refreshes_state_before_the_scope_gate_and_keeps_analysispins thatthe refresh precedes the scope gate while
AGENTS.mdguidance remains part ofthe workflow. All three fail without the fix.
Video/Screenshots
Not applicable: this change modifies prompt text and documentation only, with no
user-facing UI.
Notes
automations/catalog/github-pr-reviewer/manifest.jsonsetup.bundle.version(and the entryversion) are bumped1.4.0->1.5.0,with the matching
template.versionintests/fixtures/automations/github-pr-reviewer.json.automation-service behavior is modified.
scope decision also reflects current state.
Generated by OpenHands AI on behalf of the user. This PR was authored by all-hands-bot, so it requires human review.