fix(ci): align opencode-review's verdict poll budget with the dispatched job's own ceiling - #1532
Merged
Merged
Conversation
…hed job's own ceiling #1500, #1506 and contextual-orchestrator#968, #946 all reproduced the same pattern: the "Request current-head OpenCode review execution" repository_dispatch step in opencode-review.yml's opencode-review-target job always succeeded, but the job's own poll loop (180 attempts x 30s = 90 minutes, bounded by timeout-minutes: 100) always gave up before opencode-agent ever posted a verdict -- on every PR checked, no opencode-agent review landed at all, not even later. Traced the dispatch chain: opencode-review.yml fires a `merge-scheduler` repository_dispatch, which pr-review-merge-scheduler.yml's scan-pr-queue job picks up and (via pr_review_merge_scheduler.py's --pr-number fast path) re-dispatches an `opencode-review` event to opencode-review-dispatch.yml's own opencode-review-target job -- a *different* job that happens to share the same name, whose timeout-minutes is 325, with its "Run OpenCode PR Review model pool" step alone budgeted 205 minutes for contextual-orchestrator sidecar preflight/escalation across its free-tier candidate pool (observed directly in a noema-review preflight log probing 12 candidate models with several TimeoutErrors and stale-model 404s before landing 2 ready routes). Concretely, on .github#1500: the dispatch fired at 11:37:27Z and the poll gave up at 13:08:30Z (90m later), but the scan-pr-queue pipeline touching that exact PR had still not produced an OpenCode approval as of 13:55:04Z (2h18m after the original dispatch) -- after the poll had already failed. No contract test anywhere cross-checked these two "opencode-review-target" jobs' budgets against each other. This is a genuine timeout/budget mismatch, not a one-off flake: even with an immediately available runner, a worst-case-but-legitimate review can take up to the worker job's declared 325m ceiling, which is already ~3.6x the poll's 90m wait. Raise the poll job to timeout-minutes: 340 and its loop to 660 attempts (~330m of polling, matching the worker's own ceiling with headroom for this job's own dispatch/overhead), update the two existing pinned-value assertions in test_opencode_required_verdict_regression.py accordingly, and add a new cross-workflow contract test (test_verdict_poll_budget_covers_the_dispatched_review_jobs_own_ceiling) that fails if either workflow's budget regresses out of alignment again. Separately confirmed via GitHub Actions run history: pr-review-merge-scheduler.yml's repository_dispatch trigger alone has ~3950 total runs, with dozens still `status: queued` (never started, created_at == updated_at) at query time -- org-wide GitHub Actions concurrency saturation from the sheer fan-out of per-PR dispatches, 15/30-minute cron sweeps, and per-product hourly review-repair workflows compounds this same-name budget mismatch, though that capacity problem is out of this fix's scope (see session report for detail and follow-up recommendations). contextual-orchestrator is implicated as a contributing factor (its free-tier model pool's real per-request latency and unreliability is why worker-side timeouts are budgeted in the hundreds of minutes to begin with) but not with a new regression: recent contextual-orchestrator main commits are narrow discovery-robustness hardening, and PR #971 (removing fixed wall-clock inference deadlines) is unmerged and unrelated. Verification: coverage run -m pytest tests -- 2127 passed, 1 skipped; coverage report -- 100% on scripts/ci; interrogate -- 100% docstrings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| fi | ||
| verdict="" | ||
| for attempt in $(seq 1 180); do | ||
| for attempt in $(seq 1 660); do |
Contributor
There was a problem hiding this comment.
🔴 Queue delay still exhausts verdict poll
seq 1 660 starts before two dispatched workflows queue, leaving only five minutes beyond the worker ceiling. Longer queueing still rejects valid reviews.
Prompt for agents
The required workflow starts its fixed poll clock before the merge-scheduler workflow runs and before that scheduler dispatches opencode-review-dispatch.yml. The worker's 325-minute timeout excludes both workflows' queue and startup delays, so comparing a 329.5-minute sleep budget directly with 325 minutes does not guarantee the poll outlasts the worker. Redesign the wait contract to include bounded scheduler/queue latency, or track the exact dispatched run and wait relative to its lifecycle while keeping the requester within GitHub's job ceiling. Update tests/test_opencode_required_verdict_regression.py to model end-to-end dispatch-to-verdict time rather than comparing two isolated duration literals.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+245
to
+247
| # ContextualWisdomLab/.github#1500, #1506, contextual-orchestrator#968, | ||
| # #946: dispatch always succeeded, but the 90-minute-bounded poll below | ||
| # always gave up before the dispatched job could post a verdict). |
Contributor
There was a problem hiding this comment.
🔍 Cross-repository references are ambiguous
contextual-orchestrator#968 and the trailing #946 omit the required owner/repository form. Their GitHub links can resolve to the wrong repository.
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b24429264
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fi | ||
| verdict="" | ||
| for attempt in $(seq 1 180); do | ||
| for attempt in $(seq 1 660); do |
There was a problem hiding this comment.
When a dispatched review approaches its allowed 325-minute runtime, this 329.5-minute loop leaves only 4.5 minutes for the rest of the path. The request first dispatches pr-review-merge-scheduler.yml, whose scan-pr-queue job then dispatches opencode-review-dispatch.yml; that workflow must run validate-pr-metadata, coverage-source-tree, and coverage-evidence before its 325-minute opencode-review-target even starts. Thus ordinary queueing or prerequisite execution beyond 4.5 minutes can still make this required check fail before a legitimate verdict is posted, and the new test's comparison with only the terminal job timeout does not establish the intended end-to-end bound.
Useful? React with 👍 / 👎.
| # ContextualWisdomLab/.github#1500, #1506, contextual-orchestrator#968, | ||
| # #946: dispatch always succeeded, but the 90-minute-bounded poll below | ||
| # always gave up before the dispatched job could post a verdict). | ||
| timeout-minutes: 340 |
There was a problem hiding this comment.
When multiple PRs trigger this required workflow during the already-observed Actions queue saturation, each run now occupies an ubuntu-latest slot for up to roughly 330 minutes while mostly sleeping. The scheduler and dispatched review jobs that produce the awaited verdict also require ubuntu-latest, so enough waiter jobs can exhaust the organization's concurrency and keep those producer jobs queued until the waiters fail; increasing the timeout from 100 to 340 minutes substantially amplifies this starvation loop. The long wait should not hold a hosted runner continuously, or producer capacity must be reserved separately.
Useful? React with 👍 / 👎.
| # sidecar preflight/escalation across its free-tier candidate pool, so a | ||
| # shorter poll budget here fails a legitimate slow-but-successful review | ||
| # indistinguishably from a genuinely broken dispatch (see | ||
| # ContextualWisdomLab/.github#1500, #1506, contextual-orchestrator#968, |
There was a problem hiding this comment.
When this workflow comment is viewed on GitHub, contextual-orchestrator#968 is not the repository's required linkable cross-repository form, so the timeout evidence neither links to the issue nor creates the intended timeline cross-reference. The binding project protocol applies this convention to comments, and the same shorthand was also added to the regression-test docstring; replace both occurrences with ContextualWisdomLab/contextual-orchestrator#968.
AGENTS.md reference: AGENTS.md:L4-L4
Useful? React with 👍 / 👎.
seonghobae
pushed a commit
that referenced
this pull request
Sep 1, 2026
… after rebase Concurrent main PRs (#1532, #1533) legitimately changed .github/workflows/opencode-review-dispatch.yml since this branch's last rebase, and this rebase's merge picked those changes up byte-for-byte (confirmed: `git diff origin/main -- .github/workflows/opencode-review-dispatch.yml` is empty). Two pre-existing contract tests were left pointing at stale expectations by that upstream change -- reproducible on origin/main's own tip, not introduced by this branch's diff: - REVIEW_DISPATCH_BLOB_SHA pinned the workflow's pre-#1532/#1533 blob SHA; updated to the current `git hash-object` value. - test_opencode_privileged_review_security_boundaries_are_fail_closed asserted the pre-#1533 strict `[ "$SUPPLIED_HEAD_SHA" = "$live_head_sha" ]` equality check. #1533 ("fix(opencode): proceed on head-only advance in review dispatch validation") deliberately removed head_sha from the fail-closed mismatch list -- a head advance between dispatch capture and this job is normal PR activity that every downstream job already re-validates independently (STALE_HEAD guards), so failing closed on it only starved the required review check of a verdict. Updated the assertion to check for the new warn-and-proceed behavior instead of the old fail-closed check it replaced. Co-Authored-By: Claude <noreply@anthropic.com>
seonghobae
pushed a commit
that referenced
this pull request
Sep 1, 2026
… after rebase Same fix as #1444's identical rebase-time finding, applied here since this branch independently merged the same concurrent main PRs (#1532, #1533). Concurrent main PRs legitimately changed .github/workflows/opencode-review-dispatch.yml since this branch's last rebase, and this rebase's merge picked those changes up byte-for-byte (confirmed: `git diff origin/main -- .github/workflows/opencode-review-dispatch.yml` is empty). Two pre-existing contract tests were left pointing at stale expectations by that upstream change -- reproducible on origin/main's own tip, not introduced by this branch's diff: - REVIEW_DISPATCH_BLOB_SHA pinned the workflow's pre-#1532/#1533 blob SHA; updated to the current `git hash-object` value. - test_opencode_privileged_review_security_boundaries_are_fail_closed asserted the pre-#1533 strict `[ "$SUPPLIED_HEAD_SHA" = "$live_head_sha" ]` equality check. #1533 ("fix(opencode): proceed on head-only advance in review dispatch validation") deliberately removed head_sha from the fail-closed mismatch list -- a head advance between dispatch capture and this job is normal PR activity that every downstream job already re-validates independently (STALE_HEAD guards), so failing closed on it only starved the required review check of a verdict. Updated the assertion to check for the new warn-and-proceed behavior instead of the old fail-closed check it replaced. Co-Authored-By: Claude <noreply@anthropic.com>
seonghobae
pushed a commit
that referenced
this pull request
Sep 1, 2026
Resolves a real conflict in tests/test_opencode_required_verdict_regression.py (both branches added independent, non-overlapping tests to the same file: this branch's draft-gate regression tests, main's #1532 poll-budget-ceiling guard) by keeping both. Also fixes two issues the merge exposed: - The merged-in poll loop bound increase (180->660 attempts via #1532) turned test_non_draft_pr_without_a_verdict_still_fails_closed's real `sleep 30` calls into a ~5.5-hour test; stub `sleep` as a no-op on PATH alongside the existing fake `gh`. - main is currently red on the review-dispatch blob pin and the security-boundary test (stale since #1533's already-merged head_sha warn-and-proceed change); re-pin both to match, same fix already applied in #1482, pending #1536. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
seonghobae
pushed a commit
that referenced
this pull request
Sep 1, 2026
…e draft-gate fix main's opencode-review.yml has been substantially redesigned since this branch last synced (#1507/#1532): the old 325-minute synchronous poll loop is gone, replaced by a fast check-once-dispatch-and-fail-closed "Resolve current-head formal OpenCode verdict" step plus a separate formal-receipt "wake" callback that reruns the failed job once a verdict actually lands, instead of blocking a runner for hours. #1533's head_sha warn-and-proceed design (which an earlier revision of this fix's blob-pin port matched) was also reverted upstream (#1540, real bugs found by Codex/Devin) -- restored to the original hard-fail assertions and current blob pin. The draft-gate exemption itself is unaffected by any of that and is re-applied cleanly against the new three-step structure: - "Resolve current-head formal OpenCode verdict" now exits early with verdict=DRAFT for a draft PR, mirroring its existing closed exit. - "Request current-head OpenCode review execution"'s own if: also skips drafts, so a transient OIDC/dispatch failure can't turn a draft PR's check red before the exemption runs. - The now-trivial "Fail closed without a current-head OpenCode verdict" step (no gh calls or loop left in it at all) treats VERDICT=DRAFT the same as VERDICT=CLOSED. - converted_to_draft added to the trigger types, so a ready PR converted back to draft with no new commit still gets a fresh run. tests/test_opencode_required_verdict_regression.py's old _run_step helper and its six tests assumed the removed monolithic polling step; replaced with _run_verdict_step/_run_fail_closed_step matching the new split, keeping the same draft/closed/ready-for-review coverage. Full suite: 2216 passed, 1 skipped, 21 subtests. Ruff, interrogate, YAML, and shell-syntax checks clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
seonghobae
pushed a commit
that referenced
this pull request
Sep 1, 2026
Confirmed while verifying #1500's own fix would actually let its review pipeline run: 45 Required OpenCode Review runs sit queued simultaneously on this repo alone, one nearly two hours old. Ruled out a stale-run cancellation bug first (the concurrency group is correctly scoped by repo+PR-number with cancel-in-progress: true, and a superseded push does cancel its predecessor) -- this is raw concurrent-job demand from dozens of open PRs each restacking onto a fast-moving main, exceeding available runner capacity. Distinct from, and not fixed by, #1532's poll-budget correctness fix: that fixes the state machine, this is the state machine never getting a runner at all.
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
2 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.
Root cause (confirmed on 3 independent PRs:
.github#1500,.github#1506,contextual-orchestrator#968)Two
opencode-review-targetjobs share almost the same name but were never budget-reconciled:opencode-review.yml, this repo's required check): dispatches amerge-schedulerrepository_dispatch, then pollsfor attempt in $(seq 1 180); do ... sleep 30 ...(90 minutes), bounded bytimeout-minutes: 100.opencode-review-dispatch.yml, reached viapr-review-merge-scheduler.yml'sscan-pr-queue→pr_review_merge_scheduler.py --pr-numberfast path): its own job istimeout-minutes: 325, with theRun OpenCode PR Review model poolstep alone budgetedtimeout-minutes: 205(per the existingtest_opencode_job_timeout_contains_full_sequential_review_budgetcontract) — needed because the contextual-orchestrator sidecar preflight probes ~12 free-tier candidate models sequentially with realTimeoutErrors and stale-model 404s (directly observed in anoema-reviewpreflight log:"probed_count": 12, "ready_count": 2, "rejected_count": 10, several"error_type": "TimeoutError").The dispatch always succeeded on every PR checked. The poll never found a verdict because it structurally cannot outlast a worker job 3.6x its own budget — not a flake, and not a defect in any of the affected PRs' own diffs. Concrete timeline on
.github#1500: dispatch fired11:37:27Z, poll gave up13:08:30Z(90m later), but thescan-pr-queuepass touching that exact PR had still not produced an approval as of13:55:04Z(2h18m after dispatch — 47 minutes after the poll had already given up). Neither.github#1500norcontextual-orchestrator#968ever received anopencode-agentreview at any point.contextual-orchestratoris implicated as a contributing factor (its free-tier model pool's real per-request latency/flakiness is why worker-side timeouts are budgeted in the hundreds of minutes to begin with) but not as a new regression: recentcontextual-orchestratormaincommits are narrow discovery-robustness hardening, and open PR#971("remove fixed wall-clock deadlines from model inference") is unmerged and unrelated to this failure mode.Separately confirmed via GitHub Actions run history:
pr-review-merge-scheduler.yml'srepository_dispatchtrigger alone has ~3950 total runs, with dozens stillstatus: queued(never started) at query time — org-wide Actions concurrency saturation compounds this same-name budget mismatch, though that capacity problem is out of this fix's scope (flagged as a follow-up recommendation, not fixed here).Fix
Raise the poll job to
timeout-minutes: 340and its loop to 660 attempts (~330m of polling — matches the worker's own 325m ceiling with headroom for this job's own dispatch/overhead). Update the two existing pinned-value assertions intest_opencode_required_verdict_regression.py, and add a new cross-workflow contract test,test_verdict_poll_budget_covers_the_dispatched_review_jobs_own_ceiling, that fails if either workflow's budget ever drifts out of alignment again.Verification
coverage run -m pytest tests && coverage report --show-missing→ 2127 passed, 1 skipped, 21 subtests, 100% coverage onscripts/ci/.interrogate→ 100% docstrings.python3 -c "import yaml; yaml.safe_load(open('.github/workflows/opencode-review.yml'))"→ OK.tests/test_opencode_required_verdict_regression.py— 12 passed.Developer experience
Prevents legitimate, slow-but-successful
opencode-agentreviews from being indistinguishable from a genuinely broken dispatch — the required check now waits long enough to actually observe the outcome it's asking for.User experience
No user-facing change — CI-only fix.
Note on merge path
This fix repairs the exact mechanism (
opencode-review's own verdict poll) that this PR's own required check depends on — a genuine chicken-and-egg case: the fix cannot pass its ownopencode-reviewcheck under the old, broken budget, sincemain(not this branch) is what executes for apull_request_target-triggered required check. The repo owner has explicitly authorized bypass-merge for exactly this situation on.github#1500/#1503/#1506/#1527/#1529. Once this lands onmain, those backlogged PRs'opencode-reviewchecks should self-resolve on their next natural re-evaluation — no further bypass should be needed for them.Refs:
.github#1500,#1506,#1527,#1529,contextual-orchestrator#968,#946.Generated by Claude Code