fix(ci): recover a cancelled run that is the newest for a static head - #4517
Merged
Conversation
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.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the CI recovery script to treat a cancelled workflow run as recoverable when it is the latest run for an unchanged PR head, addressing a wedge where the required status can remain cancelled indefinitely with no newer run coming to replace it.
Changes:
- Move
cancelledhandling out of the generic “coverage” predicate and intodecideRecoverywhere run ordering (latest vs older) is available. - Add recovery logic for
latest.conclusion === "cancelled"with a new reasoncancelled_latest_run. - Extend unit tests to cover both “cancelled newest → recover” and “cancelled older under a newer run → do not recover”.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| scripts/ci-recovery.mjs | Adds explicit handling for a cancelled latest run and updates coverage documentation/logic boundaries. |
| scripts/ci-recovery.test.mjs | Updates coverage expectations and adds test cases pinning the cancelled-latest vs cancelled-superseded behaviors. |
Suppressed comments (1)
scripts/ci-recovery.mjs:195
isCoveragestill treatsconclusion: "cancelled"as coverage (run.conclusion !== APPROVAL_GATED). That contradicts the updated docstring (“cancelledis judged by POSITION … so it deliberately does not try to answer that”) and can suppress recovery in mixed cases (e.g., an approval-gated newest run plus an older cancelled run would hitruns.some(isCoverage)and returnci_present). Treatcancelledas non-coverage here so only thelatest-position check indecideRecoverycontrols it.
function isCoverage(run) {
if (run.status === "in_progress") return true;
if (run.status !== "completed") return false;
return run.conclusion !== APPROVAL_GATED;
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Follow-up to #4505, which I merged earlier today. It got the approval gate
right and one adjacent case wrong.
The gap
#4505 classified
cancelledas coverage:True in the common case. False when the cancelled run is the newest for a
head that has not moved — nothing is coming to replace it, the required context
reports
cancelledrather thansuccessforever, and the PR is wedged with nopath to green. Which is exactly the state ci-recovery exists to clear.
Observed live
PR #4514, head
02f74118ff: exactly one CI run,cancelled, head static.Blocked, with no local remedy —
gh run rerunneeds admin rights, and recoverydeclined because it believed the head was covered. Its owner eventually had to
push a bump.
The fix
Judge the latest run, not "every run is cancelled". GitHub's rollup shows
the most recent check-run per name, so an older success underneath a newer
cancellation does not unblock the PR and must not suppress recovery either.
cancelledmoves out ofisCoverageentirely. That predicate cannot see arun's position among its siblings, and position is the whole question; it now
documents that rather than pretending to answer it.
Existing exclusions unchanged:
workflow that cannot start would loop.
Tests
cancelleddrops out of the still-coverage list, and two new cases pin bothdirections:
cancelled_latest_run;22 pass, plus the workflow contract test and 1611 files wired.
Also verified against live GitHub: the scan now correctly returns 0 eligible,
because #4514's owner pushed a new head whose run is in progress. The unit tests
are what pin the wedge itself.
Closes cave-geaji.