fix(opencode-review): bound verdict-polling loop by wall clock - #1707
Merged
Conversation
…ust transport-failure count The "Fail closed without a current-head OpenCode verdict" step's polling loop was bounded only by max_poll_transport_failures (consecutive gh api transport failures) with no total wall-clock cap of its own. When a review dispatch never produces a verdict while every individual gh api call keeps succeeding, the loop polled forever, holding a live GitHub Actions runner for up to the platform's 360-minute default job timeout. Confirmed live in production: multiple "Required OpenCode Review"/"Strix Security Scan" runs stuck in this exact step for 7-20 hours (e.g. run 33509949967 on bandscope#1115 stuck 1190+ minutes), consuming enough of the org's shared Actions concurrent-job capacity to stall required-review dispatch for essentially every other open PR (thousands of queued runs across .github, contextual-orchestrator, naruon, and other repos). Adds a 3-hour (10800s) wall-clock deadline check at the top of each poll iteration -- comfortably above this org's own documented "accommodate over 2 hours per model" allowance (docs/product-goal-directive.md §8) so a legitimately slow model is never falsely failed, but well short of GitHub's 360-minute job default so a runner is reliably released. This bounds how long the CI job waits for a verdict; it does not cap the model's own reasoning/streaming time, which remains governed entirely upstream. Verified directly: extracted the real step body via PyYAML (matching this file's own existing test extraction pattern) and executed it against a stubbed gh CLI with bash 5. A never-resolving verdict now exits cleanly with a clear diagnostic at exactly the deadline instead of hanging; a verdict posted immediately still succeeds normally and is unaffected by the new check. This is an emergency direct fix authorized by the repository owner given the ongoing org-wide capacity incident (a genuine chicken-and-egg situation: this fix's own required review cannot complete because the system it fixes is what is broken). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 20 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
seonghobae
deleted the
fix/opencode-review-verdict-poll-wallclock-bound
branch
September 2, 2026 08:45
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
+439
to
+443
| poll_deadline_epoch=$(( $(date -u +%s) + 10800 )) | ||
| while :; do | ||
| if [ "$(date -u +%s)" -ge "$poll_deadline_epoch" ]; then | ||
| echo "::error::No current-head OpenCode verdict after 180 minutes of polling; failing closed and releasing the runner." | ||
| exit 1 |
Contributor
There was a problem hiding this comment.
🔴 Long reviews lose required approval
When a verdict needs over three hours, poll_deadline_epoch fails the required check before the authorized review finishes. The completed verdict cannot satisfy that run.
Prompt for agents
Replace the fixed three-hour admission deadline in .github/workflows/opencode-review.yml's current-head verdict poll with a capacity-safe design that does not impose a fixed wall-clock cutoff on an active review. The current repository contract in docs/adr/0003-contextual-orchestrator-vendored-free-zdr.md and docs/adr/0005-sidecar-preflight-token-budget.md permits hours-long model work and ends it only for operator action or an obsolete PR head. If one runner cannot remain occupied indefinitely, split polling across short-lived event-driven or redispatched runs while preserving the same exact-head verdict admission. Update the workflow execution tests to cover a verdict produced after the former three-hour boundary.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+441
to
+443
| if [ "$(date -u +%s)" -ge "$poll_deadline_epoch" ]; then | ||
| echo "::error::No current-head OpenCode verdict after 180 minutes of polling; failing closed and releasing the runner." | ||
| exit 1 |
Contributor
There was a problem hiding this comment.
🟡 Final-minute verdicts are discarded
When a verdict arrives during the final sleep, poll_deadline_epoch exits before reading it. The required check fails despite timely evidence.
Prompt for agents
If the three-hour policy remains, restructure the polling loop so reaching the deadline cannot bypass a verdict that arrived before it. Preserve live-head, state, and draft revalidation, perform a final Reviews API read at the boundary, and fail only when that read still finds no admissible current-head verdict. Add an executable test with the verdict appearing during the last sleep interval.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+439
to
+443
| poll_deadline_epoch=$(( $(date -u +%s) + 10800 )) | ||
| while :; do | ||
| if [ "$(date -u +%s)" -ge "$poll_deadline_epoch" ]; then | ||
| echo "::error::No current-head OpenCode verdict after 180 minutes of polling; failing closed and releasing the runner." | ||
| exit 1 |
Contributor
There was a problem hiding this comment.
🔍 Deadline lacks durable regression coverage
Existing loop tests omit poll_deadline_epoch, so they no longer execute production setup faithfully. Add expiration, immediate-verdict, and boundary-arrival cases.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+439
to
+441
| poll_deadline_epoch=$(( $(date -u +%s) + 10800 )) | ||
| while :; do | ||
| if [ "$(date -u +%s)" -ge "$poll_deadline_epoch" ]; then |
Contributor
There was a problem hiding this comment.
📝 Info: Epoch arithmetic is mechanically sound
On the pinned Ubuntu runner, date -u +%s and Bash arithmetic support this range. A date failure also stops the step closed.
Was this helpful? React with 👍 or 👎 to provide feedback.
seonghobae
added a commit
that referenced
this pull request
Sep 2, 2026
Preserve #1706's incident lineage while adopting protected main 6f70174 without force-push or destructive rebase. The prior 180-minute wall-clock implementation is already present on main via #1707 and is now subject to the current-head review finding; follow-up commits on this branch will replace that elapsed-time bound with the repository's existing exact-run wake continuation contract.
This was referenced Sep 2, 2026
seonghobae
added a commit
that referenced
this pull request
Sep 2, 2026
… (#1710) * fix(tests): match live-head-moved regression to #1697's intentional reorder #1697 (commit 5c561a6) reordered opencode-review.yml's live-state checks so closed/draft admission runs before the head-SHA-match check, and exits 0 instead of 1 for an open, ready PR whose live head has moved. A draft PR whose live head has moved is therefore exempted by the draft check first — the head-moved branch is now unreachable while still draft. test_opencode_live_draft_state_regression.py's test_draft_exemption_fails_closed_when_live_head_moved still asserted the pre-#1697 behavior (returncode 1, "head moved while validating live" in stdout) for exactly that input shape, so it fails on current main. Update it to assert the actual current behavior (returncode 0, exempted via the draft-check message), matching the equivalent direct-production-step coverage #1697 already added in test_opencode_required_verdict_regression.py. Confirmed via a clean origin/main worktree that the regression pre-dates this change and is not introduced by it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4 * fix(tests): stub sleep in OpenCode poll regression tests, salvage #1706 Two existing tests extract the real "Fail closed without a current-head OpenCode verdict" step's bash and run it against a fake gh, but never stubbed `sleep` -- driving the transport-failure retry path to its 3-failure threshold performed two genuine 60s sleeps per affected test run (confirmed directly: this exact gap made a 2-test run exceed a 120s timeout). Both now stub `sleep` alongside the existing fake `gh`, matching the pattern already used in test_opencode_poll_self_retirement.py: tests/test_opencode_required_verdict_regression.py::test_fail_closed_step_still_polls_for_a_non_draft_pr tests/test_opencode_live_draft_state_regression.py::test_stale_draft_verdict_event_does_not_exempt_live_ready_pr Also fixes test_opencode_poll_self_retirement.py, which was silently broken on current main: #1707's wall-clock-deadline fix to opencode-review.yml added a `poll_deadline_epoch` reference at the top of the poll loop, but this file's `_run_poll_loop` harness never declared that variable before splicing in the now-changed real loop body, so 7 of its tests failed with an empty gh-calls.log (the script aborted under `set -u` before making any call). Adds the missing `poll_deadline_epoch` line and an injectable fake `date` (extending the existing fake-gh/fake-sleep/fake-timeout harness) to prove the wall-clock deadline logic itself: the loop fails closed with the new diagnostic once the deadline is exceeded even when every gh call keeps succeeding (the exact zombie scenario the fix targets), a fast verdict is unaffected, and the production shape keeps both bounds distinct and additive. No test sleeps for real time. Full affected suite (73 tests) verified green in ~16s; the full project suite (2582 passed, 1 skipped, 21 subtests) runs in ~116s with 100% coverage and 100% docstrings, matching #1706's own claimed 236.65s -> 112.76s improvement. This is a same-file-conflict-driven successor to #1706, which also included this exact test-file delta. #1706 additionally touched .github/workflows/opencode-review.yml with the wall-clock-deadline logic itself -- that exact fix already landed separately as #1707 (bypass-merged during the org-wide capacity incident, before #1706 finished), which is why #1706 is now DIRTY/CONFLICTING against main through no fault of its own test-file changes. This PR carries only the still-valid, non-redundant test-suite-hang fix forward; #1706 is being closed in favor of this PR. Branched from and includes #1705 (fix/live-draft-regression-test-1697, a different in-flight fix to the same tests/test_opencode_live_draft_state_regression.py file, addressing an unrelated draft-head-moved logic question) to avoid a second same-file conflict. If #1705 merges to main independently before this PR, this PR's identical carried-forward hunk should merge as a no-op; if this PR merges first, #1705 should rebase onto main afterward. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
This was referenced Sep 2, 2026
seonghobae
added a commit
that referenced
this pull request
Sep 2, 2026
…l-failure fix The org's own automated PR review/fix loop had already reconciled this branch with main once (commit c946c7b), but that reconciliation silently regressed the actual headRefOid fix back to the pre-fix buggy pattern (str(pr.get("headRefOid") or "").lower()) in stale_pr_run_ids() and active_review_run_refs(), and lost the entire live-revalidation safety net (_direct_pr_run_still_superseded, _review_run_still_superseded, _cancel_revalidated_review_run_refs). Its own attempt to push a corrected commit then failed closed (correctly) when its configured push credential was unavailable, leaving the branch stuck in the regressed state. This commit resolves a fresh merge of current main (with #1707/#1702/#1704/ #1711/#1712 all applied) directly against a37a428 -- this branch's last verified-good commit (100% coverage, 2614 tests, real regression tests reproducing the naruon PR #1528 incident) -- rather than building on top of the already-regressed c946c7b. Combines both fixes at every cancellation call site (cancel_stale_pr_runs, cancel_stale_opencode_runs, _cancel_revalidated_review_run_refs): PR #1669's live revalidation immediately before each destructive cancel call (closing the TOCTOU gap a snapshot-only headRefOid check can't), and #1712's check of force_cancel_workflow_runs's actual per-run failure result (so a run proven stale but whose cancel API call GitHub itself rejected is never reported as cancelled). #1712's simpler force_cancel_workflow_run_refs wrapper is removed as dead code now that its call sites all use the more thorough per-caller revalidation; its own tests were adapted to target the functions that actually carry its safety guarantee forward, not deleted. Verified: PYTHONPATH=. coverage run -m pytest tests -- 2621 passed, 1 skipped, 21 subtests; coverage report -- 100% on scripts/ci; interrogate -- 100% docstrings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
seonghobae
added a commit
that referenced
this pull request
Sep 2, 2026
) * fix(noema-review): bound both jobs to a job-level timeout-minutes Neither cancel-closed-pr-runs nor noema-review declared a job-level timeout-minutes, so a stuck run falls back to GitHub's 360-minute platform default -- the same defect class PR #1702 fixed for scan-pr-queue. This file's own poll loops are already bounded by iteration count (unlike opencode-review.yml's pre-#1707 while :; do loop), so no wall-clock-inside-a-loop patch is needed here; the gap is purely the missing job ceiling. cancel-closed-pr-runs gets timeout-minutes: 20 -- its only step is a single-repository, status-filtered gh api --paginate list-and-cancel sweep (up to 3 passes x 5 statuses), no branch update or merge, lighter than scan-pr-queue's own timeout-minutes: 30. noema-review gets timeout-minutes: 210. Its "Prepare Noema model verdict" step calls into two_phase.py's call_llm via the same contextual-orchestrator gateway whose unbounded wait caused the 7-20 hour stuck runs PR #1707 fixed in opencode-review.yml -- noema_review_gate.py's own comment confirms that call "remains governed by contextual-orchestrator rather than a fixed inference timeout," so nothing upstream bounds it either. 210 minutes carries the same ~180-minute (3-hour) allowance PR #1707 set for its analogous model-wait deadline -- comfortably above this org's documented "모델당 두 시간 이상 걸릴 수 있음을 수용한다" policy (docs/product-goal-directive.md #8, which names Noema explicitly) -- plus a 30-minute buffer for this job's other steps (tarball fetch, credential mint, its own superseded-run cleanup sweep, visibility-lookup retries, sidecar provisioning, publication). cancel-in-progress was left as-is: this workflow's only genuinely high-frequency trigger (synchronize) already gets cancel-in-progress: true, and no evidence supports changing the lower-frequency paths. Adds test_cancel_closed_pr_runs_has_a_bounded_runtime and test_noema_review_job_has_a_bounded_runtime_above_the_two_hour_model_allowance, extracting each job's real timeout-minutes value with the same workflow_text()-based contract-test pattern this file and test_required_workflow_queue_contract.py already use. actionlint .github/workflows/noema-review.yml passes clean; tests/test_noema_orchestrator_workflow_contract.py and the full suite (2592 passed, 1 pre-existing skip) pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(noema): encode no model wall-clock timeout repair * ci(noema): materialize PR1715 timeout-authority repair --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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
1 participant
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.
Summary
Emergency direct fix, authorized by the repository owner given the ongoing org-wide GitHub Actions capacity incident.
The "Fail closed without a current-head OpenCode verdict" step's polling loop in
opencode-review.ymlwas bounded only bymax_poll_transport_failures(consecutivegh apitransport failures), with no total wall-clock cap. When a review dispatch never produces a verdict while every individualgh apicall keeps succeeding, the loop polled forever, holding a live runner for up to GitHub's 360-minute platform default job timeout.Confirmed live: multiple "Required OpenCode Review"/"Strix Security Scan" runs stuck in this exact step for 7-20 hours (e.g. run 33509949967 on bandscope#1115, stuck 1190+ minutes) — enough of the org's shared Actions concurrent-job capacity consumed to stall required-review dispatch for essentially every other open PR org-wide.
Fix
Adds a 3-hour (10800s) wall-clock deadline checked at the top of each poll iteration — comfortably above this org's own documented "accommodate over 2 hours per model" allowance (
docs/product-goal-directive.md§8), well short of GitHub's 360-minute job default. This bounds how long the CI job waits for a verdict; it does not cap the model's own reasoning/streaming time.Verification
Extracted the real step body via PyYAML (same pattern this file's own existing tests already use) and executed it directly against a stubbed
ghCLI under bash 5:This is a genuine chicken-and-egg situation (item 31 of the standing backlog): this PR's own required review cannot complete because the system it fixes is what's broken. Bypass-merge authorized by the repository owner in real time given the active incident.
🤖 Generated with Claude Code