diff --git a/.github/scripts/labels-reconcile.sh b/.github/scripts/labels-reconcile.sh index 152eb7b..048f62a 100644 --- a/.github/scripts/labels-reconcile.sh +++ b/.github/scripts/labels-reconcile.sh @@ -78,12 +78,31 @@ checks_state() { # rollup JSON on stdin → SUCCESS | FAILURE | PENDING | NONE # Once CANCELLED blocks, judging every entry would strand this very PR in # needs-rebase forever, so collapse each context to its newest entry first. # Key on workflow + name because a bare job name is only unique within its - # workflow; ordering falls back through the timestamps a pending run has. + # workflow. + # + # Dating a run is the subtle part, and getting it wrong restores the bug. + # A run still in flight has no completion, but `gh` does not omit the + # field: its Go struct marshals the zero time as "0001-01-01T00:00:00Z", + # which is a string, so `//` will not fall through it. Ordering on + # completion therefore sorted the LIVE re-run to the bottom and let `last` + # pick the very run it superseded — reporting the old SUCCESS while a + # replacement was still running, which is #136 again. + # + # So: take the newest timestamp a run actually carries, discarding both + # spellings of absent (null, and the zero sentinel). An entry that carries + # no usable timestamp at all sorts LAST rather than first — something we + # cannot date is most likely the thing just created, and treating it as + # newest keeps an undateable in-flight run from being discarded in favour + # of a stale success. Every ambiguity here resolves toward "not settled". | [ (.statusCheckRollup // [])[] | { ctx: [.workflowName // "", .name // .context // ""], - at: (.completedAt // .startedAt // .createdAt // ""), + at: ([.startedAt, .createdAt, .completedAt] + | map(select(type == "string" and . != "" + and (startswith("0001-01-01") | not))) + | max // ""), outcome: ((.conclusion // .state // "") | ascii_upcase) } ] - | group_by(.ctx) | map(sort_by(.at) | last | .outcome) as $latest + | group_by(.ctx) + | map(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.md b/CHANGELOG.md index 76ce04a..19980be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -103,7 +103,16 @@ which records not just what changed but what each drill run proved. context collapsing to its newest entry: a re-run does not evict the run it replaced, so this PR's own tip carried a `CANCELLED` `scope` beside the `SUCCESS` `scope` that superseded it, and judging every entry would have - stranded every re-run PR in `needs-rebase`. + stranded every re-run PR in `needs-rebase`. Which entry is newest is decided + on the newest timestamp a run actually carries, because a run still in flight + does not omit its completion — `gh` marshals the Go zero time as the *string* + `"0001-01-01T00:00:00Z"`, which `//` will not fall through. Dating on + completion therefore sorted the live re-run to the bottom and picked the run + it superseded, reporting the old `SUCCESS` while a replacement was still + running: #136 restored, by the very rule meant to close it. An entry carrying + no usable timestamp sorts last rather than first, so an undateable in-flight + run is never discarded in favour of a stale success — every ambiguity here + resolves toward "not settled". `UNKNOWN` mergeability is deliberately not treated as unmergeable: GitHub reports it for about a minute after every merge while it recomputes, and @@ -116,8 +125,9 @@ which records not just what changed but what each drill run proved. `CHANGELOG.md`. Queue order is intent, so the reconciler never sets it — it only **clears** it the moment the PR stops being mergeable-by-a-human, which is precisely the staleness that made `needs-human` untrustworthy. Both live - shapes, the mixed round, and the whole check-outcome enum are pinned in - `test/labels-reconcile.sh` (19 fixtures → 44). + shapes, the mixed round, the whole check-outcome enum, and the in-flight + re-run superseding both a green and a cancelled predecessor are pinned in + `test/labels-reconcile.sh` (19 fixtures → 48). - **CI's shellcheck sweep never lints `.github/scripts/*.sh`** (#116) — `globstar` makes `**` descend into subdirectories, but a glob still does diff --git a/test/labels-reconcile.sh b/test/labels-reconcile.sh index 32b0bf3..86be743 100644 --- a/test/labels-reconcile.sh +++ b/test/labels-reconcile.sh @@ -258,6 +258,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)" +# -- 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 +# completion" are pinned, because `gh` emits the zero sentinel (a string, +# which `//` does not fall through) while the API emits null. +inflight_() { jq -n --arg n "$1" --arg t "$2" --arg c "${3:-0001-01-01T00:00:00Z}" \ + '{__typename:"CheckRun", workflowName:"ci", name:$n, status:"IN_PROGRESS", + conclusion:"", startedAt:$t, completedAt:(if $c == "null" then null else $c end)}'; } + +expect "a re-run in flight beats the success it superseded (zero sentinel)" PENDING \ + "$(rollup "[$(run_ build SUCCESS 2026-07-20T15:00:00Z),\ + $(inflight_ build 2026-07-20T15:10:00Z)]" | checks_state)" +expect "...and the same when the absent completion is null" PENDING \ + "$(rollup "[$(run_ build SUCCESS 2026-07-20T15:00:00Z),\ + $(inflight_ build 2026-07-20T15:10:00Z null)]" | checks_state)" +expect "a replacement in flight for a CANCELLED run is pending, not failed" PENDING \ + "$(rollup "[$(run_ build CANCELLED 2026-07-20T15:00:00Z),\ + $(inflight_ build 2026-07-20T15:10:00Z)]" | checks_state)" +# an entry carrying no usable timestamp is treated as newest, not oldest — +# ambiguity resolves toward "not settled" rather than toward a stale success +expect "an undateable in-flight run is not discarded for a stale success" PENDING \ + "$(rollup "[$(run_ build SUCCESS 2026-07-20T15:00:00Z),\ + $(jq -n '{__typename:"CheckRun",workflowName:"ci",name:"build",conclusion:"",startedAt:null,completedAt:null}')]" \ + | checks_state)" + # -- the classifier feeds the state machine: a cancelled required check must # take the PR off the human's plate, which is the whole point of #136. DRAFT=false HEAD_SHA=head1 REQUESTED="$HUMAN" REVIEWS_JSON="$ALL_APPROVE" MERGEABLE=MERGEABLE