fix(scheduler): retire a dispatch status its own check run superseded - #2061
fix(scheduler): retire a dispatch status its own check run superseded#2061seonghobae wants to merge 1 commit into
Conversation
`opencode-review-dispatch.yml` publishes the `opencode-review` commit status once, from whichever dispatch reaches the publish step first, and its own step calls that status optional while naming the exact-head required review authoritative. When the dispatch loses the race it records `failure` minutes before the required `opencode-review` check run concludes `success` on the very same head, and no later run ever rewrites the status. `failed_status_checks` already lets a successful status context retire a failing check run of the same name. It had no rule for the opposite direction, so the stale `failure` status stayed in the failing set forever: OpenCode's evidence gate reads a failed same-head check, publishes CHANGES_REQUESTED, and the pull request is permanently BLOCKED behind a verdict its own authoritative check run had already superseded. Measured live on 2026-09-09 against ContextualWisdomLab/.github: of the 69 open non-draft pull requests, 36 carried exactly this pair -- `opencode-review` status `failure` with the exact-head `opencode-review` check run `success`. Example: #2013 at e9402a9, status `failure` published 06:22:20Z by run 34187701578, check run `success` at 06:41:19Z in run 34182950404, single status in the context's whole history. The new rule keys on the required-context name, so a sibling job of the OpenCode workflow succeeding cannot retire a genuine `opencode-review` failure, and it routes through `is_opencode_context`, which keeps it inert in central required-workflow mode where the CheckRun is the non-authoritative placeholder and the status is the real verdict. Fail-closed is preserved: `merge_approval_block_reason` still independently requires GitHub `reviewDecision == APPROVED` plus a non-author formal APPROVED review on the exact current head, and a genuinely failing check run is still reported when no successful `opencode-review` check run exists on that head. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 41 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: Advanced Run ID: 📒 Files selected for processing (2)
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 |
Correction to this PR's stated impactThe description's closing line — "the 36 affected pull requests become eligible to progress" — overstates the effect. Measured live just now, every one of the sampled contradiction PRs carries at least one additional failing check, most commonly
So this change removes one of two or more blockers, it does not by itself make those PRs mergeable. What it does fix is precise and still worth landing on its own:
The correctness argument in the description is unchanged, and the fail-closed analysis stands: Scope note: this PR deliberately does not touch the publisher side ( 🤖 Generated with Claude Code |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head product diff. Coverage is a separate gate.
Changed files
scripts/ci/pr_review_merge_scheduler_core.py— review and security gate shell pathtests/test_pr_review_merge_scheduler.py— regression suite
Changed behavior
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["CI script: pr_review_merge_scheduler_core.py"]
S1 --> I1["review and security gate shell path"]
I1 --> R1["Review risk: CI script: pr_review_merge_scheduler_core.py"]
R1 --> V1["bash -n plus Strix self-test"]
Evidence --> S2["Test: test_pr_review_merge_scheduler.py"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: test_pr_review_merge_scheduler.py"]
R2 --> V2["targeted test run"]
Findings
No source-backed product finding is synthesized from the coverage gate. A coverage miss belongs in the status comment.
- Head SHA:
a04052a86298eb05201449379e8349b32e85df7a - Workflow run: 34346312308
- Workflow attempt: 1
- Coverage gate:
failure
Review outcome
Coverage is a gate, not the review. This body reviews the changed product files.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["CI script: pr_review_merge_scheduler_core.py"]
S1 --> I1["review and security gate shell path"]
I1 --> R1["Review risk: CI script: pr_review_merge_scheduler_core.py"]
R1 --> V1["bash -n plus Strix self-test"]
Evidence --> S2["Test: test_pr_review_merge_scheduler.py"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: test_pr_review_merge_scheduler.py"]
R2 --> V2["targeted test run"]
OpenCode Review Overview
|
There was a problem hiding this comment.
Pull request overview
OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.
Findings
1. HIGH Current-head GitHub Checks - Fix failed required checks before approval
- Problem: Failed same-head checks remain for
a04052a86298eb05201449379e8349b32e85df7a. - Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
- Fix: Read and fix the failed check logs below, then rerun the current-head checks.
- Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.
Failed checks:
- Required Noema Review/noema-review: FAILURE (https://github.com/ContextualWisdomLab/.github/actions/runs/34345594903/job/102446624943)
- noema-review check run: failure (https://github.com/ContextualWisdomLab/.github/actions/runs/34345594903/job/102446624943)
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["CI script: pr_review_merge_scheduler_core.py"]
S1 --> I1["review and security gate shell path"]
I1 --> R1["Review risk: CI script: pr_review_merge_scheduler_core.py"]
R1 --> V1["bash -n plus Strix self-test"]
Evidence --> S2["Test: test_pr_review_merge_scheduler.py"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: test_pr_review_merge_scheduler.py"]
R2 --> V2["targeted test run"]
1 participant
Problem
52% of the open non-draft pull requests in this repository are permanently
BLOCKEDbehind a verdict their own authoritative check run already superseded.opencode-review-dispatch.ymlpublishes theopencode-reviewcommit status once, from whichever dispatch reaches the publish step first. That step's own text calls the status optional and names the exact-head formal review authoritative. When the dispatch loses the race it recordsfailureminutes before the requiredopencode-reviewcheck run concludessuccesson the very same head — and no later run ever rewrites the status, because the dispatch that wrote it has already exited.failed_status_checksalready lets a successful status context retire a failing check run of the same name (pr_review_merge_scheduler_core.py:2550). It had no rule for the opposite direction. So the stalefailurestatus stays in the failing set forever: OpenCode's evidence gate reads a failed same-head check, publishesCHANGES_REQUESTED, and the PR never leavesBLOCKED.Evidence (live, 2026-09-09)
Of the 69 open non-draft PRs, 36 carry exactly this pair —
opencode-reviewstatusfailurewith the exact-headopencode-reviewcheck runsuccess:failure/ check runsuccess(this bug)failure(genuinely failing)failure/ check runfailureWorked example — #2013 at
e9402a91:opencode-reviewstatus =failure, created06:22:20Z, run 34187701578, description "No validated exact-current-head OpenCode approval was published (model outcome: exhausted)."opencode-reviewcheck run =success, completed06:41:19Z, run 34182950404GET /commits/e9402a91.../statusesreturns that failure as the only status in the context's entire history — it is terminal, 19 minutes stale on arrival.Change
One mirrored rule in
failed_status_checks: a failingopencode-reviewstatus context is skipped when aSUCCESSopencode-reviewcheck run exists on the same head.coverage-source-tree,coverage-evidence, …) cannot retire a genuineopencode-reviewfailure.is_opencode_context, which returnsFalsefor CheckRuns whenSCHEDULER_REQUIRED_WORKFLOW_REPOSITORYis set — so this stays inert in central required-workflow mode, where the CheckRun is the non-authoritative placeholder and the status is the real verdict. Production.githubsets that variable nowhere (only tests do), so the check run is authoritative here, as classic branch protection expects.Why this cannot wrongly merge anything
Fail-closed is preserved on every remaining boundary:
merge_approval_block_reasonstill independently requires GitHubreviewDecision == APPROVEDandhas_independent_current_head_approval— a non-author formalAPPROVEDreview on the exact current head. Neither is affected by this change.opencode-reviewis still reported whenever no successfulopencode-reviewcheck run exists on that head (asserted in the new test's second case, which still returns both failures).SUCCESScontradicts. It cannot invent a pass.Tests
New
test_successful_opencode_check_run_supersedes_a_stale_dispatch_status— RED before the change (['opencode-review', 'lint'] != ['lint']), GREEN after, and pins the negative case so the suppression cannot widen.Developer experience: a PR whose review actually passed stops presenting a contradictory permanent failure, so
gh pr checksand the scheduler agree on one verdict instead of two.User experience: the 36 affected pull requests become eligible to progress through the review→merge loop again instead of requiring a manual bypass each.
🤖 Generated with Claude Code