fix(github): retry errored deliveries and wait on action_required heads by all-hands-bot · Pull Request #688 · OpenHands/extensions

all-hands-bot · 2026-09-27T16:14:09Z

HUMAN:

AGENT:

Why

While debugging why the Issue Triage and PR Reviewer automations were not covering their backlogs, two defects surfaced that strand work permanently rather than merely delay it.

AgentConversationDispatcher.deliver() resumes a stable, per-subject conversation and short-circuits when the stored delivery matches. A conversation whose execution_status is ERROR falls into the catch-all and is reported as deduplicated, so a conversation that died mid-task is never restarted. The delivery string is stable for a subject, so that subject stays stranded. Observed in production: an outstanding all-hands-bot review request whose conversation had execution_status: error, and every later scan returned deduplicated.
The reviewer head gate accepts only success/neutral/skipped as non-blocking, so a completed run with conclusion action_required is classified as blocked. GitHub parks a fork head's workflow runs at action_required until a maintainer approves them, so a head that only awaits approval is reported as failing.

Summary

Re-applied on current main on 2026-10-04 (main split the dispatcher into an OpenHands API path and a local Agent Server path, and reworked the gate, since this PR was opened):

skills/github/scripts/agent_conversation.py (shared by the reviewer, issue-to-PR, and issue-triage automations): a matched delivery whose conversation is in ERROR is sent again and reported as retried, on both delivery paths. Retries are bounded at two per delivery (error_retries in the KV record, reset by a new delivery), so a conversation that fails the same way every time is not re-run on every scan. A retry also counts against the scan's max_new_per_run launch bound, like a new conversation. On the OpenHands API path, the status of an errored conversation whose sandbox is already paused cannot be seen, so that case still reports deduplicated (documented in the code).
skills/github-pr-reviewer/scripts/worker.py: a completed action_required run is waiting, listed as <name> (awaiting maintainer approval), and the gate comment asks a maintainer to approve the workflow runs instead of saying no action is needed (this addresses the earlier review on this PR). The blocked heading says checks "did not pass" rather than "failed", since blocked also covers cancelled, timed-out, and unknown conclusions. Unknown conclusions still fail closed.
Bundle versions bumped for the automations that ship these files: github-pr-reviewer 1.9.2 (after fix(pr-review): do not restart completed reviews on delayed submitted events #647's 1.9.1), github-issue-to-pr 1.2.2, github-issue-triage 1.3.1; automations/bundle-index.js regenerated.

Explicit all-hands-bot review requests already bypass the gate on main, so the action_required change affects scheduled and label-triggered reviews.

Issue Number

#653

How to Test

New and changed tests:

tests/test_agent_conversation_dispatch.py: test_errored_conversation_retries_its_delivery, test_errored_conversation_stops_retrying_after_the_bound, test_new_delivery_resets_the_error_retry_count, test_cloud_run_retries_a_delivery_whose_conversation_errored, test_cloud_run_stops_retrying_an_errored_delivery_after_the_bound.
tests/test_github_reviewer_delivery.py: test_reviewer_waits_on_a_run_awaiting_maintainer_approval (check-run shape) and test_reviewer_waits_on_a_fork_workflow_run_awaiting_approval (the real fork shape: a workflow run with no check runs), test_a_retried_conversation_uses_a_launch_slot, and test_reviewer_fails_closed_on_an_unknown_conclusion now uses a genuinely unknown conclusion.

The new dispatcher and gate tests fail against main's code and pass with this change.

$ uv run pytest -q tests/test_agent_conversation_dispatch.py tests/test_github_reviewer_delivery.py
113 passed

$ uv run pytest -q
1153 passed, 23 skipped

$ npm run build && uv run python scripts/sync_extensions.py --check && git diff --check
(no drift; only the pre-existing non-blocking coverage warning for ./plugins/issue-duplicate-checker)

Video/Screenshots

Not applicable - no GUI change.

Notes

STUCK conversations still report deduplicated; with the retry bound in place they could be handled the same way in a follow-up.
No live deployment was exercised for this update.
_classify_required_runs cannot show the approval note: the required-check set only contains contexts that already reported, so a fork head where some required check reported is classified by those alone. That predates this PR and is worth a follow-up issue.

This PR was created by an AI agent (OpenHands) on behalf of the user, and re-applied on current main by an AI agent (Claude Code, Opus 5.5) helping Engel Nyst (@enyst).

Two defects in the GitHub automation scripts that strand work permanently. 1. agent_conversation.deliver() resumes a stable per-subject conversation and short-circuits when the stored delivery matches. An execution status of ERROR fell into the catch-all arm and was reported as 'deduplicated', so a conversation that died mid-task was never retried: the delivery string is stable for a subject, so the subject stayed stranded with its work unfinished. Observed in production as an outstanding reviewer request whose dispatch record existed, whose conversation had errored, and which no later scan would ever restart. ERROR now re-sends the prompt and retries, reported as 'retried'. 2. The reviewer head gate in worker.py treated a completed run with conclusion 'action_required' as a failure, because it only accepted success/neutral/ skipped as non-blocking. GitHub parks a fork head's workflows at 'action_required' until a maintainer approves them; the check runs those workflows would create do not exist yet, and the underlying check runs are green. A head only awaiting approval was therefore misreported as red and blocked indefinitely. It now classifies as waiting, and the blocked gate comment no longer claims the checks 'failed' when they did not pass for a reason other than failure. Adds a regression test for the retry path and converts the action_required fails-closed test into a waiting assertion.

The automation bundle index embeds the shipped script contents, so the agent_conversation.py and reviewer worker changes require regenerating it with npm run build:automations.

all-hands-bot

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

Scope

The change lives in this repository's own extension content (skills/github-pr-reviewer/scripts/worker.py, skills/github/scripts/agent_conversation.py) plus the regenerated automations/bundle-index.js, which is exactly where AGENTS.md says reusable reviewer/dispatch behavior and automation bundle scripts belong. It is not a cross-repository or product/architecture decision. Linked issue #653 is open and still carries ready-for-dev with priority:low; this change matches the issue's requested "retry the errored delivery" direction.

What I verified

Head is still 5c22f42ee06e23550df7123344f385ab20cbcceb; state open, not draft, labeled type: fix.
Focused modules (test_agent_conversation_dispatch.py, test_github_reviewer_delivery.py, plus triage/developer/watchdog/handoff/foundation) pass: 157 passed.
Full suite passes: 1070 passed, 23 skipped.
The derived automations/bundle-index.js embeds both new code paths (retried, action_required, did not pass), so the generated catalog is in sync with the hand-authored source.
Head check rollup: all required checks green. The rollup also lists an earlier Validate PR description failure from run 36332573292 that was superseded at the same head by a later Validate PR description success (16:17). That is a stale rerun, not a code defect.

The two core fixes are sound: routing a matched delivery whose conversation is ERROR to a re-send instead of deduplicated is correct, and the retry correctly rewrites the KV record (retried is not short-circuited). Classifying a completed action_required run as waiting rather than blocked is also a fairer reading than calling it red.

Findings

One user-facing correctness gap on the newly-routed action_required state, detailed inline. action_required is now classified as waiting, but the idle-waiting gate body tells the reader that no action is needed and that the scan retries "once every check on the head reports a conclusion". An action_required run already reports a conclusion and cannot advance to success without a maintainer approving it, so the automated comment describes a retry condition that is already satisfied while the head is permanently stalled.

Non-blocking note: ConversationExecutionStatus.STUCK is terminal (like ERROR) but still falls through to deduplicated in the same branch, so a conversation that got stuck mid-task remains stranded under the same stable-delivery reasoning. Not required by issue #653, but worth a follow-up if the ERROR case is considered fixed here.

🔄 CHANGES REQUESTED

all-hands-bot · 2026-09-29T10:21:49Z

            status = (run.get("status") or "").lower()
-            if status != "completed":
+            conclusion = (run.get("conclusion") or "").lower()
+            if status == "completed" and conclusion == "action_required":


action_required is now routed into the waiting arm, but the waiting gate body (_gate_body, the non-blocked branch) tells the reader "No action is needed. The scheduled scan retries once every check on the head reports a conclusion." An action_required run already reports a conclusion, and it will not advance to success on its own - GitHub holds the run until a maintainer approves it. So a fork head in this state is permanently stalled while the automated comment tells maintainers no action is needed, and the retry condition it names is already satisfied. The blocked wording was fixed in this PR, but the same inaccuracy now surfaces through the waiting body. Consider giving the approval-pending case its own wording that names the actual unblock (maintainer approval of the parked workflow run) instead of reusing the in-progress waiting text.

…quired-gate Main reworked the dispatcher (cloud and local delivery paths) and the reviewer gate since this branch was opened. Take main's version here; the next commit re-applies the fix on top of it.

… on main Port both fixes onto the current dispatcher and reviewer gate. - A matched delivery whose conversation is in ERROR is sent again and reported as `retried` on both the local Agent Server path and the OpenHands API path. Retries are bounded (two per delivery, counted in the KV record and reset by a new delivery), so a conversation that fails the same way every time is not re-run on every scan. - A completed run with conclusion `action_required` is waiting, not blocking. It is listed as "(awaiting maintainer approval)", and the gate comment asks a maintainer to approve the workflow runs instead of saying no action is needed, which addresses the review on this PR. The fail-closed test now uses a genuinely unknown conclusion. - Bump the bundles that ship these files: github-pr-reviewer 1.9.2 (after #647's 1.9.1), github-issue-to-pr 1.2.2, github-issue-triage 1.3.1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

A retried conversation runs again, so the scan's launch bound now counts it like a new one; otherwise an outage that errors every review would restart all of them on the next scan. Document that the OpenHands API path cannot see an errored conversation whose sandbox is already paused, and test action_required in its real fork shape: a workflow run with no check runs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…quired-gate

enyst

I'm an AI agent (Claude Code, based on Opus 5.5) helping Engel Nyst (@enyst) with project work.

Approving f9996b6. main split the dispatcher into an OpenHands API path and a local Agent Server path after this was opened, so I merged main in (including #647) and re-applied both fixes:

Errored deliveries: a matched delivery whose conversation is in ERROR is retried on both paths. Retries are bounded (two per delivery, reset by a new delivery) and count against the scan's max_new_per_run, so an outage that errors every review can't restart all of them at once.
action_required runs: these now count as waiting, and the gate comment asks a maintainer to approve the workflow runs. This addresses the earlier review's point that the comment said no action was needed. Unknown conclusions still fail closed.

An independent pass reviewed the port. Its findings, retries bypassing the launch bound and the fork-shaped action_required case (a workflow run with no check runs), are fixed and tested. Full suite: 1153 passed, 23 skipped. Required checks are green.

Validate PR description fails only because the HUMAN: note is empty. This PR has no human author, and the template says agents must not write that section, so I left it as is.

- A footer stamped by the worker is kept after the agent profile is edited, and a raising llm_provenance still lets the verdict and maintainer handoff run, with the failure logged as JSON. - A delivery retried after an ERROR (#688) renders the prompt for the conversation it is sent to, so its review marker still names it. - Document the worker's stamp-once, non-blocking provenance. - Bump github-pr-reviewer to 1.11.0, github-issue-to-pr to 1.2.4, and github-issue-triage to 1.3.3 over main's 1.9.2/1.2.2/1.3.1; regenerate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The shipped worker, main.py, and shared dispatcher changed, and the setup form gained `maxInFlight`. Main is at 1.9.0, and the open #647 and #688 take 1.9.1 and 1.9.2, so this takes the next free minor. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Bring in #647 and #688. Mechanical resolution: - agent_conversation.py: keep both constant groups (Cloud sandbox states and `_MAX_ERROR_RETRIES`); `_deliver_local` takes both the PR's `subject` and #688's `can_retry`; `deliver` writes #688's `error_retries` into the record, then keeps the PR's `_kv_put` and registry bookkeeping. The retry branches are not gated by the in-flight cap yet; that follows in a separate commit. - worker.py: `ReviewIntake.drain` keeps the PR's stop on `deferred` and #688's count of `created` and `retried` against the per-scan bound. - test_agent_conversation_dispatch.py: keep #688's Cloud retry tests and the PR's Cloud in-flight section. - github-pr-reviewer manifest and fixture: keep the PR's 1.10.0 over main's 1.9.2. - automations/bundle-index.js regenerated with `npm run build`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

#688 re-sends a matched delivery whose conversation ended in ERROR, which runs the conversation again. On both delivery paths that retry now passes the same admission as a start or resume: - `_deliver_local` and `_deliver_cloud` reserve a slot before the retry and return `deferred` when the deployment is at its cap. - `deliver` returns a deferral before the `error_retries` bookkeeping and the record write, so a deferred retry keeps its delivery unconsumed and does not spend one of its bounded attempts. - A retry that raises while starting releases its slot. `ReviewIntake.drain` still stops at the first `deferred` result before it counts `created` and `retried` against the per-scan bound. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

openhands-release-bot · 2026-10-05T01:19:40Z

🚀 Released in v0.28.0.

Bring the dev branch up to OpenHands/extensions 0.28.0, which is 16 commits ahead and includes upstream fixes to the GitHub PR reviewer itself: OpenHands#647 stops a delayed submitted review event from restarting a completed review, OpenHands#688 retries errored deliveries and waits on action_required heads, and OpenHands#652 requires live UI evidence before approval. Those land in the shared reviewer the compound-engineering entry reuses, so dev was carrying known bugs until this. Merged rather than rebased: fork/dev is what agent_canvas_extensions_ref points the running server at, so its history is deployed. A rebase would rewrite a published ref and force a push onto the branch the server is reading. Three files needed resolution. README.md conflicted only in its auto-generated catalog section, and the conflict was the extension counts, so the markers were dropped and sync_extensions.py regenerated the section authoritatively rather than hand-picking a side: 78 extensions, which is the union of upstream's new simplified-technical-english skill and our compound-engineering plugin. marketplaces/openhands-extensions.json and automations/bundle-index.js auto-merged, both additive and in different regions. uv run pytest -q: 1163 passed, 23 skipped. sync_extensions.py --check exits 0 and the catalog and bundle indexes are regenerated, so the derived files match their sources on the merged tree.

* fix: show LLM provenance in automation output Co-authored-by: openhands <openhands@all-hands.dev> * fix: make automation provenance and publication reliable Address profile fallback and successful-review footer findings from #547. Fail clearly if linked credentials need the accompanying Agent Server fix. Co-authored-by: openhands <openhands@all-hands.dev> * fix: identify fallback settings without assuming the active profile The active profile pointer can drift from the concrete default LLM settings. Label 404 fallback as default instead of claiming the named profile was read. Co-authored-by: openhands <openhands@all-hands.dev> * fix: use active settings for LLM provenance Co-authored-by: openhands <openhands@all-hands.dev> * fix: honor the selected automation LLM profile Resolve AUTOMATION_MODEL for both scripts and preserve the complete LLM configuration. Fall back only for an absent or missing selection; report the actual default model without assuming the active-profile pointer. Add HTTP regressions, regenerate bundle 1.0.3, and explain selection, credential handling, and the server dependency in .pr/selected-profile.html. Co-authored-by: openhands <openhands@all-hands.dev> * fix(review): bind LLM provenance to the conversation that wrote the review The catalog reviewer launches worker.py, which delegates through AgentConversationDispatcher and called the shared prompt builder without provenance, so every review asked for `default` / `unknown`. The legacy footer repair also matched "our" review by bot login, head, and time, which lets an overlapping run on the same bot rewrite another run's review. - The prompt now carries a hidden run marker naming the receiving conversation (``) instead of asking the agent for a footer. The dispatcher accepts a prompt function of the conversation id, so the marker names the conversation actually created or resumed, including a cloud replacement. - The deterministic side appends `LLM profile: <profile> · Model: <model>` only to reviews whose single marker names its own conversation. The worker reads the model back from that conversation and the LLM profile from its launching agent profile at the launch revision (`unknown` once edited); it never uses the scanner's settings. The legacy poller chooses the conversation id up front and uses the LLM config it started it with. - The worker reads the verdict above the appended footer, so approval and maintainer handoff still work. - Slack and legacy records without provenance report `unknown`, not `default`. - Bump github-pr-reviewer to 1.10.0 and the two other bundles shipping agent_conversation.py (issue triage 1.3.1, issue to PR 1.2.2); regenerate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: remove the stale selected-profile design note The 14 September note described only the AUTOMATION_MODEL scripts, the timestamp-based footer verification, and reviewer bundle 1.0.3. The catalog reviewer now starts conversations from an agent profile and binds its footer to the review's run marker, so the note no longer describes this PR. Its still-valid points (selection and fallback rules, the Agent Server #4952 dependency for provider-linked profiles) are in the README and PR notes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(review): stamp provenance once and never let it block completion A review that already carries the footer keeps it, so a later edit of the agent profile cannot rewrite what was recorded. A failure while reading provenance is logged instead of propagating into the completion path, so the verdict and maintainer handoff still run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(review): cover once-only stamping and retried prompts; bump bundles - A footer stamped by the worker is kept after the agent profile is edited, and a raising llm_provenance still lets the verdict and maintainer handoff run, with the failure logged as JSON. - A delivery retried after an ERROR (#688) renders the prompt for the conversation it is sent to, so its review marker still names it. - Document the worker's stamp-once, non-blocking provenance. - Bump github-pr-reviewer to 1.11.0, github-issue-to-pr to 1.2.4, and github-issue-triage to 1.3.3 over main's 1.9.2/1.2.2/1.3.1; regenerate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: keep automation LLM provenance display-only Restore the existing settings loaders and remove named-profile resolution and provider-credential requirements. Record only the model already used by each script, and keep catalog attribution tied to its conversation. Co-authored-by: openhands <openhands@all-hands.dev> * docs(.pr): show the display-only provenance flow Co-authored-by: openhands <openhands@all-hands.dev> * docs(.pr): add live issue display evidence * docs(.pr): keep hosted evidence links usable * test: give the provenance fixture a high-priority closing issue Main now hands a review off to a maintainer only when the pull request closes a same-repository issue labeled priority:medium or priority:high (#748). The provenance tests that check an approval still hands off after the footer used a pull request with no closing issue, so they no longer reached the handoff. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: Remove PR-only artifacts [automated] * fix: record trusted reviewer provenance before preserving footers * fix: keep review attribution accurate across failures and ACP profiles * fix: reconcile saved attribution after failed metadata reads * fix: coordinate unknown review footers with the recorded tuple * Use run workspace for local review conversations * Regenerate automation bundles for local workspace fix * chore: refresh PR comparison No files changed. This commit refreshes GitHub's pull request comparison so we can verify the catalog version diff against current main. * fix(automations): use next issue-to-PR bundle version Set the shipped bundle version to 1.2.3 over main's 1.2.2. Keep the top-level template version at 1.2.2 because this PR does not change that catalog's prompt, form, or filter. --------- Co-authored-by: openhands <openhands@all-hands.dev> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: allhands-bot <allhands-bot@users.noreply.github.com>

github-actions Bot added the type: fix A bug fix label Sep 27, 2026

chore(automations): regenerate bundle index

5c22f42

The automation bundle index embeds the shipped script contents, so the agent_conversation.py and reviewer worker changes require regenerating it with npm run build:automations.

all-hands-bot commented Sep 29, 2026

View reviewed changes

enyst and others added 4 commits October 4, 2026 21:43

Merge upstream/main into openhands/fix-errored-delivery-and-action-re…

47dbaf8

…quired-gate Main reworked the dispatcher (cloud and local delivery paths) and the reviewer gate since this branch was opened. Take main's version here; the next commit re-applies the fix on top of it.

Merge upstream/main into openhands/fix-errored-delivery-and-action-re…

f9996b6

…quired-gate

enyst approved these changes Oct 4, 2026

View reviewed changes

enyst merged commit 038dbfa into main Oct 4, 2026
12 of 14 checks passed

enyst deleted the openhands/fix-errored-delivery-and-action-required-gate branch October 4, 2026 20:08

openhands-release-bot Bot mentioned this pull request Oct 4, 2026

chore(main): release 0.28.0 #728

Merged

enyst mentioned this pull request Oct 4, 2026

fix(review): bound in-flight conversations across scheduled scans #690

Open

openhands-release-bot Bot added the released: v0.28.0 Shipped in v0.28.0 label Oct 5, 2026

Navigation Menu

Repository navigation

fix(github): retry errored deliveries and wait on action_required heads - #688

fix(github): retry errored deliveries and wait on action_required heads#688
enyst merged 6 commits into
mainfrom
openhands/fix-errored-delivery-and-action-required-gate

all-hands-bot commented Sep 27, 2026 •

edited by enyst

Loading

Uh oh!

all-hands-bot left a comment

Uh oh!

all-hands-bot Sep 29, 2026

Uh oh!

enyst left a comment

Uh oh!

Uh oh!

openhands-release-bot Bot commented Oct 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

all-hands-bot commented Sep 27, 2026 • edited by enyst Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

Summary

Issue Number

How to Test

Video/Screenshots

Notes

Uh oh!

all-hands-bot left a comment

Choose a reason for hiding this comment

Scope

What I verified

Findings

Uh oh!

all-hands-bot Sep 29, 2026

Choose a reason for hiding this comment

Uh oh!

enyst left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

openhands-release-bot Bot commented Oct 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

all-hands-bot commented Sep 27, 2026 •

edited by enyst

Loading