fix(ci): stop reading an approval-gated run as CI coverage - #4505
Merged
Conversation
GitHub parks a first-time-contributor run behind its manual-approval gate. That
run reports status=completed with conclusion=action_required and ZERO jobs, so
the pull request shows no checks at all and never will until someone approves.
decideRecovery classified purely on `status`, so `completed` read as finished CI
and returned ci_present — recovery never fired and Copilot-authored PRs sat
BLOCKED with an empty check rollup.
The root cause is one layer deeper than "the check ignores conclusion":
parseRun never captured `conclusion` at all, so the decision could not have
consulted it even in principle. It is now parsed, validated as string-or-null
(null while a run is queued or in progress), and returned.
Scoped narrowly and deliberately. A FAILED run is coverage — it reported a
verdict, and re-dispatching would just re-run a known failure. A `cancelled`
run is normally superseded by a newer push carrying its own run. A
`startup_failure` genuinely produced no coverage, but re-dispatching a workflow
that cannot start would loop, so it is left to a human. Only the approval gate
is both no-coverage AND fixed by a fresh dispatch, so only it recovers, under
the new reason `approval_gated_run`.
Recovery also requires EVERY run on the head to be gated: one real run beside a
gated one still counts as coverage, so a redundant dispatch cannot be triggered
by a stray gated run.
Tests cover all three: the gated head recovers, four real conclusions
(success, failure, cancelled, timed_out) stay coverage, and a mixed head does
not recover.
Note for the next editor: decideRecovery is `async`, and an anchor of
"function decideRecovery(" matches the tail of that declaration. Splicing text
at it orphans the `async` and breaks the `await` inside — which is exactly what
happened once while writing this.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes CI recovery incorrectly treating GitHub’s manual-approval–gated workflow runs as “CI coverage”, which previously prevented auto-recovery dispatches for PRs whose only run was parked behind approval (completed/action_required with zero jobs).
Changes:
- Parse and surface
workflow_run.conclusioninparseRun, validating it as string-or-null. - Refine recovery classification via a new
isCoverage(run)helper and a new recovery reason:approval_gated_run(only when all head runs are gated). - Add test coverage for approval-gated runs, ensuring other conclusions continue to count as coverage and that mixed heads don’t recover.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/ci-recovery.mjs | Captures conclusion, introduces approval-gate detection, and adjusts recovery decision logic to avoid false “ci_present” on gated runs. |
| scripts/ci-recovery.test.mjs | Extends fixtures with conclusion and adds regression tests for the approval-gated scenario and related edge cases. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The comment claimed conclusion is "a string once it completes", but the validation accepts string-or-null against any status. Reviewer flagged the mismatch on #4505. Keep the lenient validation and correct the comment. A completed run that reports no conclusion is odd, not malformed, and the only consumer asks whether the conclusion IS the approval gate — null answers "no" and the run counts as coverage. Throwing there would turn an API quirk into a dead recovery tool.
BunsDev
added a commit
that referenced
this pull request
Aug 10, 2026
…#4517) cave-qshvl (#4505) classified `cancelled` as coverage, reasoning that a cancelled run is normally superseded by a newer push carrying its own run. That holds right up until the cancelled run is the NEWEST one for a head that has not moved: nothing is coming to replace it, the required context reports `cancelled` rather than `success` forever, and the PR is wedged with no path to green. Which is precisely the state ci-recovery exists to clear. Observed on #4514: head 02f7411 carried exactly one CI run, cancelled, and the head had not moved. `pnpm ci:recovery` reported "5 open PRs scanned; 0 eligible" while the PR sat blocked, and no local remedy existed — `gh run rerun` requires admin rights, and recovery declined because it saw coverage. Decided against the LATEST run rather than "every run is cancelled", because GitHub's rollup shows the most recent check-run per name: an older success underneath a newer cancellation does not unblock the PR, so it must not suppress recovery either. The existing exclusions are unchanged. A failure is still coverage — it reported a verdict. A startup_failure is still left to a human, since re-dispatching a workflow that cannot start would loop. `cancelled` moves out of isCoverage entirely: that predicate cannot see a run's position among its siblings, and position is the whole question here. It now says so rather than pretending to answer. Tests: `cancelled` drops out of the still-coverage list, and two new cases pin both directions — cancelled-and-newest recovers, cancelled-underneath-a- newer-run does not. 22 pass. Verified against live GitHub state, where the scan correctly returns 0 eligible now that #4514's owner has pushed a new head carrying an in-progress run.
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
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.
Closes cave-qshvl.
The bug
GitHub parks a first-time-contributor run behind its manual-approval gate. That run reports
status: "completed"withconclusion: "action_required"and zero jobs — so the PR shows no checks at all, and never will until someone approves it.decideRecoveryclassified purely onstatus:completedread as finished CI, so recovery never fired and Copilot-authored PRs satBLOCKEDwith an empty check rollup. The empty-run detection below that branch was unreachable for this case, since it only applies whenstatus === "queued".The root cause was one layer deeper
Not merely "the check ignores
conclusion" —parseRunnever capturedconclusionat all, so the decision could not have consulted it even in principle. It is now parsed, validated as string-or-null (null while a run is queued or in progress), and returned.Scoped narrowly, on purpose
Only the approval gate recovers, under a new reason
approval_gated_run. The other non-success conclusions are deliberately left as coverage, and the reasoning is in the code so nobody assumes they were overlooked:failure— reported a verdict. That is coverage; re-dispatching just re-runs a known failure.cancelled— normally superseded by a newer push that carries its own run; recovering it would fight that.startup_failure— genuinely no coverage, but re-dispatching a workflow that cannot start would loop, so it is left to a human.Recovery also requires every run on the head to be gated. One real run beside a gated one still counts as coverage, so a stray gated run cannot trigger a redundant dispatch.
Tests
approval_gated_runsuccess,failure,cancelled,timed_out— all remain coverage and do not recoverThe
workflowRunfixture gained aconclusionfield defaulting tosuccessfor completed runs, so existing tests keep asserting the same behavior.Verification
scripts/ci-recovery.test.mjsscripts/ci-recovery-workflow.test.mjspnpm check:tests-wiredgit diff --checkNote for the next editor
decideRecoveryisasync, and the string"function decideRecovery("matches the tail of that declaration. Splicing text at that anchor orphans theasyncand breaks theawaitinside — which is exactly what happened once while writing this, andnode --checkon the test file passes while the module is the broken one.Scope note
The bead cites PR #4487 as the live example. That PR has since resolved — it now has 20 check-runs and a successful run — so the symptom is no longer observable there. The defect in the code is unchanged and was verified directly:
ci-recovery.mjshad zero references to.conclusionbefore this change.