From 67cb0e05d75317613caa91897fdf13c19d40b837 Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Wed, 5 Aug 2026 11:50:27 +0200 Subject: [PATCH 1/2] feat(pr-status): tell a queued check apart from a running one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- evals/evals.json | 6 ++++ .../references/pull-request-workflow.md | 16 +++++++++ skills/git-workflow/scripts/pr-status.sh | 34 ++++++++++++++++--- 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/evals/evals.json b/evals/evals.json index 92fd0fb..9c0cb09 100644 --- a/evals/evals.json +++ b/evals/evals.json @@ -150,6 +150,12 @@ "name": "already-enqueued-pr-reads-as-clean", "prompt": "I queued my PR for merge ten minutes ago. The status check now says mergeState CLEAN, every check green, no open threads, NEXT: merge. Should I run the merge again?", "expected_output": "Should say no and check the PR's own queue entry first: mergeStateStatus describes the branch, so an enqueued PR keeps reading CLEAN and looks merge-ready; mergeQueueEntry (position, state, estimatedTimeToMerge) is the field that distinguishes 'ready to enqueue' from 'already in line', and a repo-level merge_queue ruleset does not answer it; should wait for the queue to merge it and note that enqueueing again only restarts its checks" + }, + { + "id": 26, + "name": "required-check-queued-not-running", + "prompt": "My PR has been waiting on CI for forty minutes and the status tool says the required checks are still running. Should I enqueue it for merge so it goes in as soon as they finish?", + "expected_output": "Should check whether those checks are actually running or merely queued: queued and in_progress are different API states, and a required check sitting in queued with nothing running means no runner has picked it up, not that CI is slow; should warn that a merge queue drops an entry whose required check does not report inside check_response_timeout_minutes and that the clock covers waiting for a runner, so enqueueing now risks a silent dequeue and each retry leaves more runs holding slots; should not try to look up the concurrency limit, which is not exposed by the API" } ] } diff --git a/skills/git-workflow/references/pull-request-workflow.md b/skills/git-workflow/references/pull-request-workflow.md index 11cca7d..7e7e9a1 100644 --- a/skills/git-workflow/references/pull-request-workflow.md +++ b/skills/git-workflow/references/pull-request-workflow.md @@ -1356,6 +1356,22 @@ netresearch/ofelia this piled up 18 unfinished runs and turned a four-attempt loop into a guaranteed failure, while the same required checks concluded in **2.0 minutes** in PR context, well inside the window. +**Tell a busy pool from a slow one before deciding.** `queued` and +`in_progress` are different states and the API reports them separately; +collapsing both into "pending" reads as "CI is running" when in truth no runner +has picked anything up. `pr-status.sh` keeps them apart — the checks line shows +`N pending (X running, Y queued)` — and answers `await-capacity` instead of +`wait` when a *required* check is queued while nothing at all is running. That +is the precondition for the paragraph above: enqueueing in that state is what +gets the entry dropped. + +The concurrency limit itself is not readable. `GET /orgs/{org}/actions/hosted-runners/limits` +answers `404 "GitHub hosted runners are not supported for this organization"` +outside the larger-runner product, and the per-plan ceiling for standard runners +exists only in the documentation. So do not try to compare against a number — +observe the state instead: required checks queued with zero running is +sufficient on its own, and needs no knowledge of the plan. + Gate on it instead: enqueue when the repo has at most ~2 unfinished runs. ```bash diff --git a/skills/git-workflow/scripts/pr-status.sh b/skills/git-workflow/scripts/pr-status.sh index 82a6986..f8ab6cd 100755 --- a/skills/git-workflow/scripts/pr-status.sh +++ b/skills/git-workflow/scripts/pr-status.sh @@ -102,7 +102,13 @@ evaluate() { | ($p.commits.nodes[0].commit.oid) as $head | ([$p.commits.nodes[0].commit.statusCheckRollup.contexts.nodes[]? | if .__typename == "CheckRun" - then {name, state: (if .status != "COMPLETED" then "PENDING" + # QUEUED and IN_PROGRESS are kept apart. Collapsing both into + # "pending" reads as "CI is running" when in truth nothing has + # started — and that is a different situation with a different + # answer: a merge queue drops an entry whose required check never + # starts, so "wait" is the wrong advice. + then {name, state: (if .status == "QUEUED" then "QUEUED" + elif .status != "COMPLETED" then "PENDING" elif .conclusion == "SUCCESS" then "PASS" elif .conclusion == "SKIPPED" or .conclusion == "NEUTRAL" then "SKIP" else "FAIL" end), url: .detailsUrl} @@ -124,9 +130,13 @@ evaluate() { | ([$head_reviews[] | select(.author.login | test("copilot"; "i"))]) as $copilot_on_head | ([$p.reviewThreads.nodes[]? | select(.isResolved == false)]) as $unresolved | ($checks | map(select(.state=="FAIL"))) as $failing - | ($checks | map(select(.state=="PENDING"))) as $pending + | ($checks | map(select(.state=="QUEUED"))) as $queued + | ($checks | map(select(.state=="PENDING"))) as $running + # "pending" downstream keeps meaning "not finished", queued or running. + | ($queued + $running) as $pending | ($failing | map(select(.name as $n | $required | index($n)))) as $failing_required | ($pending | map(select(.name as $n | $required | index($n)))) as $pending_required + | ($queued | map(select(.name as $n | $required | index($n)))) as $queued_required | { repo: $repo.nameWithOwner, number: $p.number, title: $p.title, state: $p.state, draft: $p.isDraft, @@ -138,10 +148,13 @@ evaluate() { pass: ($checks|map(select(.state=="PASS"))|length), fail: ($failing|length), pending:($pending|length), + queued: ($queued|length), + running:($running|length), skip: ($checks|map(select(.state=="SKIP"))|length), failing: ($failing|map(.name)), failing_required: ($failing_required|map(.name)), pending_required: ($pending_required|map(.name)), + queued_required: ($queued_required|map(.name)), failing_urls: ($failing|map(.url)) }, required_contexts: $required, @@ -230,6 +243,17 @@ evaluate() { then "; the existing APPROVED review sits on an older commit and this repo does not dismiss it" else "" end)), cmd:"gh api repos/\($s.repo)/pulls/\($s.number)/requested_reviewers -X POST -f \"reviewers[]=copilot-pull-request-reviewer[bot]\""} + # A required check that is QUEUED with nothing running is not "CI is + # slow" — no runner has picked it up. It is reported separately + # because the answer differs: waiting is right for a running check, + # while a queued one that never starts gets a merge-queue entry + # dropped, and the operator wants to know that before enqueueing. + elif (($s.checks.queued_required|length) > 0 and $s.checks.running == 0) then + {action:"await-capacity", + why:("required check(s) queued and not started: \($s.checks.queued_required|join(", "))" + + " — \($s.checks.queued) queued, 0 running, so no runner has picked them up." + + " Enqueueing now risks the merge queue dropping the entry when its" + + " required check never starts")} elif ($s.checks.pending_required|length) > 0 then {action:"wait", why:"required check(s) still running: \($s.checks.pending_required|join(", "))"} elif ($s.mergeState == "CLEAN" @@ -249,7 +273,9 @@ evaluate() { # pending, not only when one is red. Calling that "a non-required # check is red" sends the reader hunting for a failure that does # not exist — nothing here has failed yet. - {action:"wait", why:"UNSTABLE while \($s.checks.pending) non-required check(s) are still running — nothing has failed"} + {action:"wait", + why:("UNSTABLE while \($s.checks.pending) non-required check(s) have not finished" + + " (\($s.checks.running) running, \($s.checks.queued) queued) — nothing has failed")} elif $s.mergeState == "UNSTABLE" then {action:"triage-ci", why:"UNSTABLE: a non-required check is red; the gate stays shut until it is green or the PR is force-merged"} elif $s.checks.pending > 0 then @@ -265,7 +291,7 @@ render() { "PR #\(.number) \(.title)", " state : \(.state)\(if .draft then " (DRAFT)" else "" end) mergeable=\(.mergeable) mergeState=\(.mergeState)", " head : \(.headOid[0:8]) on \(.head) -> \(.base)", - " checks : \(.checks.pass) pass, \(.checks.fail) fail, \(.checks.pending) pending, \(.checks.skip) skip (of \(.checks.total))", + " checks : \(.checks.pass) pass, \(.checks.fail) fail, \(.checks.pending) pending\(if .checks.pending > 0 then " (\(.checks.running) running, \(.checks.queued) queued)" else "" end), \(.checks.skip) skip (of \(.checks.total))", (if (.checks.failing|length) > 0 then " failing : \(.checks.failing|join(", "))" else empty end), (if (.checks.failing_required|length) > 0 then " ^ REQUIRED : \(.checks.failing_required|join(", "))" else empty end), " rulesets : \(if (.rulesets|length)>0 then (.rulesets|join(", ")) else "none" end)", From 23cd00ed35fea14acb0ac69a42592146b464151c Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Wed, 5 Aug 2026 13:39:22 +0200 Subject: [PATCH 2/2] fix(pr-status): age the queued signal before calling it a capacity problem 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 --- skills/git-workflow/scripts/pr-status.sh | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/skills/git-workflow/scripts/pr-status.sh b/skills/git-workflow/scripts/pr-status.sh index f8ab6cd..4151c22 100755 --- a/skills/git-workflow/scripts/pr-status.sh +++ b/skills/git-workflow/scripts/pr-status.sh @@ -79,7 +79,7 @@ collect() { commits(last:1){ nodes{ commit{ oid statusCheckRollup{ state contexts(first:100){ nodes{ __typename - ... on CheckRun{ name conclusion status detailsUrl } + ... on CheckRun{ name conclusion status detailsUrl startedAt } ... on StatusContext{ context state targetUrl } } } } } } } } @@ -111,7 +111,7 @@ evaluate() { elif .status != "COMPLETED" then "PENDING" elif .conclusion == "SUCCESS" then "PASS" elif .conclusion == "SKIPPED" or .conclusion == "NEUTRAL" then "SKIP" - else "FAIL" end), url: .detailsUrl} + else "FAIL" end), url: .detailsUrl, started: .startedAt} else {name: .context, state: (if .state == "SUCCESS" then "PASS" elif .state == "PENDING" then "PENDING" else "FAIL" end), url: .targetUrl} @@ -137,6 +137,16 @@ evaluate() { | ($failing | map(select(.name as $n | $required | index($n)))) as $failing_required | ($pending | map(select(.name as $n | $required | index($n)))) as $pending_required | ($queued | map(select(.name as $n | $required | index($n)))) as $queued_required + # Right after a push every check is queued and none is running, which is + # normal for a few seconds and says nothing about runner capacity. Age the + # signal before acting on it, using the tolerance the merge queue itself + # applies (check_response_timeout_minutes, default 5) as the threshold: + # queued longer than the queue would wait is when it stops being fresh. + # No apostrophes in here — the whole jq program sits in a single-quoted + # shell string, and one would end it. + | (($queued_required | map(.started // empty) | min) // null) as $oldest_q + | (if $oldest_q == null then 0 + else (((now - ($oldest_q | fromdateiso8601)) / 60) | floor) end) as $queued_minutes | { repo: $repo.nameWithOwner, number: $p.number, title: $p.title, state: $p.state, draft: $p.isDraft, @@ -155,6 +165,7 @@ evaluate() { failing_required: ($failing_required|map(.name)), pending_required: ($pending_required|map(.name)), queued_required: ($queued_required|map(.name)), + queued_minutes: $queued_minutes, failing_urls: ($failing|map(.url)) }, required_contexts: $required, @@ -248,9 +259,10 @@ evaluate() { # because the answer differs: waiting is right for a running check, # while a queued one that never starts gets a merge-queue entry # dropped, and the operator wants to know that before enqueueing. - elif (($s.checks.queued_required|length) > 0 and $s.checks.running == 0) then + elif (($s.checks.queued_required|length) > 0 and $s.checks.running == 0 + and $s.checks.queued_minutes >= 5) then {action:"await-capacity", - why:("required check(s) queued and not started: \($s.checks.queued_required|join(", "))" + why:("required check(s) queued \($s.checks.queued_minutes) min and not started: \($s.checks.queued_required|join(", "))" + " — \($s.checks.queued) queued, 0 running, so no runner has picked them up." + " Enqueueing now risks the merge queue dropping the entry when its" + " required check never starts")}