Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 58 additions & 15 deletions .github/workflows/opencode-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -419,8 +419,55 @@ jobs:
echo "Event draft snapshot is stale; continuing verdict polling for the live ready PR."
fi
verdict=""
live_poll_failures=0
review_poll_failures=0
max_poll_transport_failures=3
while :; do
reviews="$(gh api --paginate "repos/${TARGET_REPOSITORY}/pulls/${PR_NUMBER}/reviews")"
if ! live_poll_pr="$(timeout 30s gh api "repos/${TARGET_REPOSITORY}/pulls/${PR_NUMBER}")"; then
Comment thread
seonghobae marked this conversation as resolved.
live_poll_failures=$((live_poll_failures + 1))
if [ "$live_poll_failures" -ge "$max_poll_transport_failures" ]; then
echo "::error::Live pull request read failed ${live_poll_failures} consecutive times while polling; failing closed and releasing the runner."
exit 1
fi
echo "::warning::Live pull request read failed while polling (${live_poll_failures}/${max_poll_transport_failures}); retrying after revalidation delay."
sleep 30
continue
fi
live_poll_failures=0
live_poll_head="$(printf '%s' "$live_poll_pr" | jq -r '.head.sha // empty')"
live_poll_draft="$(printf '%s' "$live_poll_pr" | jq -r 'if (.draft | type) == "boolean" then (.draft | tostring) else empty end')"
live_poll_state="$(printf '%s' "$live_poll_pr" | jq -r 'if (.state | type) == "string" then .state else empty end')"
if [ -z "$live_poll_head" ] || [ -z "$live_poll_draft" ] || [ -z "$live_poll_state" ]; then
echo "::error::Could not validate live pull request state while polling for a current-head OpenCode verdict."
exit 1
fi
if [ "$live_poll_state" != "open" ] && [ "$live_poll_state" != "closed" ]; then
echo "::error::Could not validate live pull request state while polling for a current-head OpenCode verdict."
exit 1
fi
if [ "${live_poll_head,,}" != "${HEAD_SHA,,}" ]; then
echo "::notice::Pull request head moved while waiting for a current-head OpenCode verdict; retiring superseded Required OpenCode Review poll."
exit 1
fi
if [ "$live_poll_state" = "closed" ]; then
echo "PR closed while waiting for the current-head OpenCode verdict; the poll is no longer required."
exit 0
fi
if [ "$live_poll_draft" = "true" ]; then
echo "PR became draft while waiting for the current-head OpenCode verdict; the poll is no longer required until it is marked ready for review."
exit 0
fi
if ! reviews="$(timeout 30s gh api --paginate "repos/${TARGET_REPOSITORY}/pulls/${PR_NUMBER}/reviews")"; then
review_poll_failures=$((review_poll_failures + 1))
if [ "$review_poll_failures" -ge "$max_poll_transport_failures" ]; then
echo "::error::Reviews API read failed ${review_poll_failures} consecutive times while polling; failing closed and releasing the runner."
exit 1
fi
echo "::warning::Reviews API read failed while polling (${review_poll_failures}/${max_poll_transport_failures}); revalidating live PR state before retry."
sleep 30
continue
fi
review_poll_failures=0
verdict="$(printf '%s\n' "$reviews" | jq -r -s --arg sha "$HEAD_SHA" '
(add // [])
| [
Expand Down Expand Up @@ -461,19 +508,15 @@ jobs:
echo "Current-head OpenCode verdict: ${verdict}."

cancel-superseded-opencode-review-runs:
# Scoping the concurrency group above by exact head SHA (so a delayed
# old-head run can no longer cancel the authoritative newer-head run --
# Devin Review on `#1568`) also means a *legitimate* new commit no
# longer auto-cancels its own PR's now-obsolete previous-head poll: that
# older run's own live-head check only ran once, before it entered its
# unbounded Reviews API wait, and nothing in that wait loop re-validates
# the head. Left alone, it would occupy a runner until GitHub's own
# per-job ceiling. This job retires it directly, mirroring the
# live-head-validated cleanup pattern in strix.yml's own
# `cancel-superseded-pr-runs` job: every cancellation candidate and
# every cancellation itself is re-verified against the live PR head
# immediately beforehand, so a run for this job that is itself somehow
# delayed/stale cannot wrongly cancel a still-authoritative run.
# Exact-head concurrency protects a newer authoritative run from delayed
# old-head events, while the poll above now revalidates live PR identity on
# every wait iteration so an already-running obsolete poll can self-retire
# without consuming a second runner. This sibling job remains a defense in
# depth for queued/requested old-head runs and for legacy runs created from
# older workflow revisions that lack the in-loop self-retirement check.
# Every cancellation candidate and every cancellation itself is re-verified
# against the live PR head immediately beforehand, so a cleanup run that is
# itself delayed/stale cannot cancel a still-authoritative run.
if: github.event_name == 'pull_request_target' && github.event.action == 'synchronize'
runs-on: ubuntu-24.04
permissions:
Expand Down Expand Up @@ -555,4 +598,4 @@ jobs:
for active_status in queued in_progress requested waiting pending; do
cancel_runs "$active_status"
done
echo "Superseded OpenCode review run cleanup completed."
echo "Superseded OpenCode review run cleanup completed."
37 changes: 37 additions & 0 deletions docs/doctoring/opencode-stale-poll-self-retirement.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# OpenCode stale-poll self-retirement

## Incident boundary

On 2026-09-01 UTC (2026-09-02 Asia/Seoul), `ContextualWisdomLab/fast-mlsirm` retained an in-progress `Required OpenCode Review` run for PR #1519 on predecessor head `5453d0df84e4e...` while the live PR head had already advanced to `3a3865f40da12211898c97cbd47e7460381736ae`. The predecessor run had entered the required workflow's Reviews API wait and continued occupying a runner. At the same observation, the repository had a fresh current-head OpenCode run queued and the organization-wide Actions fleet was heavily queued.

The protected central workflow intentionally keys concurrency by repository, PR number, and exact head SHA. That protects a newer authoritative run from a delayed old-head event, but it also means a new commit cannot cancel the previous head through the concurrency group. A separate `cancel-superseded-opencode-review-runs` job exists for that cleanup, yet it needs its own runner. Under saturation, the cleanup job can therefore wait behind the stale poll it is meant to retire.

## Root cause

`opencode-review-target` validated the live PR head/state/draft once before entering an unbounded `while` loop. The loop then queried only the Reviews API every 30 seconds. A head movement after the first validation was invisible to the occupied run, so an obsolete head could remain in progress until GitHub's job ceiling even though it could never receive an authoritative current-head verdict.

This is a control-plane capacity defect, not a reason to shorten semantic-review inference deadlines. A fixed short `timeout-minutes` would trade one failure mode for another and can kill legitimate long-running review work.

## Repair contract

The polling loop now re-fetches the live pull request before every Reviews API read. It:

- fails closed when live head/state/draft evidence is missing or malformed;
- exits non-passing when the live head no longer equals the workflow's immutable `HEAD_SHA`, allowing the stale run to release its runner itself;
- exits successfully when the PR closes or becomes Draft while the same head is waiting, because no verdict is required in those states;
- keeps exact-head formal `APPROVED` / `CHANGES_REQUESTED` review evidence as the only terminal substantive verdict; and
- retains the existing no-short-timeout contract for legitimate semantic reviews.

The sibling cancellation job remains defense in depth for queued/requested predecessor runs and for legacy workflow revisions that do not contain the in-loop self-retirement check.

## Regression evidence

`tests/test_opencode_poll_self_retirement.py` was committed before the production workflow change. The protected-main workflow did not contain the required in-loop live-state lookup, so the new contract is RED on `main@7d707b8abbb8a3fed95d0efe4121ed9b4f76bb2a`. The production repair follows on the same single-writer branch and is constrained to the existing required-workflow entrypoint plus the new regression and this doctoring record.

Hosted exact-head evidence remains authoritative for merge. Queue, predecessor, cancelled, skipped, or locally reasoned evidence is not promoted to a passing required check or formal review.

## Rollback and observability

Rollback is the ordinary revert of the workflow repair if exact-head evidence shows false retirement of an authoritative run. During operation, inspect the live PR head together with the workflow run's immutable head SHA. An old-head run that remains in progress after a poll interval indicates either a legacy workflow revision or a failure before the self-retirement loop; do not classify a queued replacement verdict as success.

After protected integration, re-observe affected leaf repositories. Acceptance requires predecessor-head OpenCode polls to release runner capacity without waiting for a separate cleanup runner, while unchanged current-head semantic reviews remain able to run beyond arbitrary short deadlines.
Loading
Loading