From d8f54aab04c44d34281875826c7895ed489e0b5b Mon Sep 17 00:00:00 2001 From: claude-bot-andresmgsl Date: Fri, 24 Jul 2026 12:34:51 +0000 Subject: [PATCH] fix: a queue-cancelled duplicate check is not a verdict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit checks_state discards a CANCELLED entry only when its context group holds at least one non-cancelled sibling — before the sort, so the duplicate the repo-global reconcile queue evicted (after it had already attached a check to the head) cannot outvote the success that did its work (#136 a17e497, #133 4002924). An all-cancelled context never reported at all and still classifies FAILURE; {FAILURE older, CANCELLED newest} keeps its red. The fixture that pinned the opposite rule imagined a cancelled run replacing a success; it never saw one that replaced nothing. Rewritten with its reason, plus the recorded a17e497 shape, the all-cancelled groups, and the cancelled-over-FAILURE case. Closes #139 Co-Authored-By: Claude Fable 5 --- actions/labels-reconcile/labels-reconcile.sh | 24 +++++++++++++- changelog.d/139.md | 1 + test/labels-reconcile.test.sh | 35 +++++++++++++++++++- 3 files changed, 58 insertions(+), 2 deletions(-) create mode 100644 changelog.d/139.md diff --git a/actions/labels-reconcile/labels-reconcile.sh b/actions/labels-reconcile/labels-reconcile.sh index d02e6aa..a9d2728 100755 --- a/actions/labels-reconcile/labels-reconcile.sh +++ b/actions/labels-reconcile/labels-reconcile.sh @@ -204,6 +204,22 @@ checks_state() { # rollup JSON on stdin → SUCCESS | FAILURE | PENDING | NONE | # the exact shape of #136. The cost of being wrong is symmetric in form and # not in consequence: a false FAILURE parks the PR on the agent, who looks; # a false SUCCESS invites a human to merge a tree that will not merge. + # + # The list-what-passes rule has exactly one carve-out, and it is narrower + # than an outcome: a CANCELLED entry is discarded when its context holds at + # least one non-cancelled sibling (#139). The reconcile job queues in one + # repo-global concurrency group, so any repo event — a sibling PR's push, + # triage labelling an issue — evicts the queued duplicate AFTER it has + # attached a check run to this PR's head, and that cancelled entry became + # the context's newest word: blocker:ci-red on a PR whose real checks were + # all green (#133/#136, evictable only by an empty commit). A cancelled run + # said nothing about this head; a non-cancelled sibling is a real verdict + # about exactly these bytes, whatever order the two arrived in — and for + # this workflow the evictor performs the duplicate's work anyway, since + # every sweep covers every open PR. This does not widen unknown-into-green: + # a context whose entries are ALL cancelled never reported at all (a killed + # or timed-out required job), so it keeps CANCELLED and still blocks — + # discard needs a surviving verdict, never an empty context. jq -r ' if (has("statusCheckRollup") | not) then "UNREADABLE" else @@ -252,8 +268,14 @@ checks_state() { # rollup JSON on stdin → SUCCESS | FAILURE | PENDING | NONE | and (startswith("0001-01-01") | not))) | first // ""), outcome: ((.conclusion // .state // "") | ascii_upcase) } ] + # The #139 carve-out (header above): drop CANCELLED entries only when the + # context keeps a non-cancelled survivor — BEFORE the sort, so a cancelled + # entry that arrived newest cannot outvote the real verdict it displaced. + # An all-cancelled context is left intact and still classifies FAILURE. | group_by(.ctx) - | map(sort_by([(.at == ""), .at]) | last | .outcome) as $latest + | map( map(select(.outcome != "CANCELLED")) as $live + | (if ($live | length) > 0 then $live else . end) + | sort_by([(.at == ""), .at]) | last | .outcome ) as $latest | if ($latest | length) == 0 then "NONE" elif (($latest - $passing - $waiting) | length) > 0 then "FAILURE" diff --git a/changelog.d/139.md b/changelog.d/139.md new file mode 100644 index 0000000..16d9533 --- /dev/null +++ b/changelog.d/139.md @@ -0,0 +1 @@ +- `labels-reconcile` — a queue-cancelled duplicate check is discarded when its context holds a real verdict, so a sibling PR's eviction no longer reds a green PR; an all-cancelled context still blocks (#139). diff --git a/test/labels-reconcile.test.sh b/test/labels-reconcile.test.sh index 4cb1c14..197654c 100755 --- a/test/labels-reconcile.test.sh +++ b/test/labels-reconcile.test.sh @@ -363,7 +363,15 @@ expect "NEUTRAL and SKIPPED are not failures" SUCCESS \ expect "a re-run supersedes the cancelled original" SUCCESS \ "$(rollup "[$(run_ scope CANCELLED 2026-07-20T15:19:39Z),\ $(run_ scope SUCCESS 2026-07-20T15:19:45Z)]" | checks_state)" -expect "...and the reverse order is not a re-run passing, it is one failing" FAILURE \ +# The reverse order once pinned FAILURE — "the reverse order is not a re-run +# passing, it is one failing". That fixture imagined a cancelled run REPLACING +# a success; #139 recorded a cancelled run that replaced NOTHING: the +# repo-global reconcile queue keeps one pending run per group, so a sibling +# PR's event evicts the queued duplicate after it has already attached a +# check to this head. A run that never executed a step said nothing about +# these bytes — it is not a verdict, and the success beside it is. The +# survivor decides, whatever order the two arrived in (#139). +expect "a cancelled entry beside a success is not a verdict, the success is" SUCCESS \ "$(rollup "[$(run_ scope SUCCESS 2026-07-20T15:19:39Z),\ $(run_ scope CANCELLED 2026-07-20T15:19:45Z)]" | checks_state)" # same job name in a different workflow is a different context, not a re-run @@ -371,6 +379,31 @@ expect "same name in another workflow does not supersede" FAILURE \ "$(rollup "[$(jq -n '{__typename:"CheckRun",workflowName:"labels",name:"scope",conclusion:"FAILURE",completedAt:"2026-07-20T15:00:00Z"}'),\ $(run_ scope SUCCESS 2026-07-20T15:19:45Z)]" | checks_state)" +# -- the #139 carve-out, pinned by the recorded shape that bought it. PR #136 +# head a17e497: the reconcile succeeded 12:16:17→12:17:06, and the queued +# duplicate — evicted by ANOTHER PR's run in the same repo-global group — +# attached CANCELLED at 12:16:41, started_at == completed_at, no step ever +# ran. Recorded payloads, not live fetches: the heads have moved on. +rec_() { jq -n --arg o "$1" --arg s "$2" --arg c "$3" \ + '{__typename:"CheckRun", workflowName:"labels", name:"reconcile", + conclusion:$o, startedAt:$s, completedAt:$c}'; } +expect "a queue-cancelled duplicate beside the success that did its work (a17e497)" SUCCESS \ + "$(rollup "[$(rec_ SUCCESS 2026-07-24T12:16:17Z 2026-07-24T12:17:06Z),\ + $(rec_ CANCELLED 2026-07-24T12:16:41Z 2026-07-24T12:16:41Z)]" | checks_state)" +# ...but the discard needs a surviving verdict. A context that is ONLY +# cancelled never reported at all — a killed or timed-out required job — and +# certifying that green is the unknown-as-green shape checks_state refuses. +expect "a context whose only entry is CANCELLED still blocks" FAILURE \ + "$(rollup "[$(run_ b CANCELLED)]" | checks_state)" +expect "...and so does a context of two cancelled entries" FAILURE \ + "$(rollup "[$(rec_ CANCELLED 2026-07-24T12:16:17Z 2026-07-24T12:16:20Z),\ + $(rec_ CANCELLED 2026-07-24T12:16:41Z 2026-07-24T12:16:41Z)]" | checks_state)" +# ...and discarding the cancelled entry must never discard a real red: the +# survivor rule keeps the FAILURE, it does not resurrect anything green. +expect "a cancelled newest over an earlier FAILURE is still that failure" FAILURE \ + "$(rollup "[$(rec_ FAILURE 2026-07-24T12:16:17Z 2026-07-24T12:17:06Z),\ + $(rec_ CANCELLED 2026-07-24T12:16:41Z 2026-07-24T12:16:41Z)]" | checks_state)" + # -- a run still IN FLIGHT. `run_()` cannot express this: it always carries a # real completedAt, which is exactly why the supersede rule shipped dating # runs by completion and nothing caught it. Both spellings of "no