feat(pr-status): tell a queued check apart from a running one - #139
Merged
Conversation
GitHub reports queued and in_progress as different states; pr-status collapsed both into pending and said 'required check(s) still running'. That reads as CI being slow when in truth no runner has picked anything up, and the two situations have opposite answers. The distinction decides whether enqueueing is safe. A merge queue drops an entry whose required check does not report within check_response_timeout_minutes, and that clock covers the wait for a runner — references/pull-request-workflow.md already warned not to enqueue into a busy pool, but the tool operators actually follow could not tell a busy pool from a slow one. Observed today on netresearch/t3x-nr-llm#594: both queue entries had CI at status=queued, never started, and the entry was discarded having executed nothing. The new verdict await-capacity fires when a required check is queued while nothing is running. The checks line now reads 'N pending (X running, Y queued)'. The limit itself stays out of it: /orgs/{org}/actions/hosted-runners/limits answers 404 outside the larger-runner product and the per-plan ceiling is documentation only, so the check observes state rather than comparing against a number it cannot read. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
github-actions
Bot
added
documentation
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
…oblem Review of the previous commit found a false positive it would produce on every push: for the first seconds after a push all checks are queued and none is running, which is normal and says nothing about runner capacity. The condition as written answered await-capacity immediately, telling the reader no runner had picked anything up when the checks were simply new. The verdict now also requires the oldest queued REQUIRED check to have sat there for at least five minutes, and reports the age. Five is not arbitrary: it is check_response_timeout_minutes, the tolerance the merge queue itself applies before dropping an entry, so the warning starts exactly where the risk it warns about starts. Also: the comment that carried this explanation contained an apostrophe, which ended the single-quoted shell string holding the whole jq program. shellcheck caught it (SC1011). Prose inside that string stays apostrophe-free, and the comment now says so. Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
Member
Author
|
siehe oben |
|
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.
GitHub reports
queuedandin_progressas different check states.pr-status.shcollapsed both into "pending" and answered required check(s) still running — which reads as CI being slow when in truth no runner has picked anything up. The two situations have opposite answers.Why the distinction decides something
A merge queue drops an entry whose required check does not report within
check_response_timeout_minutes, and that clock covers the wait for a runner, not just the run.references/pull-request-workflow.mdalready carried the warning — "Do not enqueue into a busy runner pool", including the detail that a dropped entry does not cancel its runs so each retry makes it worse. But the tool an operator actually follows had no way to tell a busy pool from a slow one, so the warning could not be acted on.Observed today on netresearch/t3x-nr-llm#594: both merge-queue entries had
CIatstatus=queued, never started, and the entry was discarded having executed nothing. Meanwhile 44 runs sat queued across 14 repositories with zero running.pr-statusreported "still running" throughout.The change
QUEUEDis classified separately fromIN_PROGRESS;pendingdownstream keeps its meaning of "not finished".await-capacitywhen a required check is queued while nothing at all is running, naming the checks and the counts.N pending (X running, Y queued).checks.queued,checks.runningandchecks.queued_requiredare exposed in--json.Deliberately not included: the limit
I looked for it rather than assuming:
The endpoint covers larger runners only, and the per-plan ceiling for standard runners exists in documentation, not in the API. So the check observes state — required checks queued with zero running is sufficient on its own — instead of comparing against a number it cannot read.
Verification
shellcheckandbash -nclean. Behaviour unchanged on three live PRs (triage-ci,fix-ci, and the merge-queue branch on #594 which still takes precedence, as it should).The new branch could not be exercised against live data: the backlog drained while this was being written, so no PR currently has a queued required check. The classification and the branch condition are shown on synthetic input instead, which is weaker evidence and worth saying plainly: