diff --git a/.github/scripts/labels-reconcile.sh b/.github/scripts/labels-reconcile.sh index 0c23879..f2e4dbe 100644 --- a/.github/scripts/labels-reconcile.sh +++ b/.github/scripts/labels-reconcile.sh @@ -88,18 +88,28 @@ checks_state() { # rollup JSON on stdin → SUCCESS | FAILURE | PENDING | NONE # 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". + # So: date a run by when it BEGAN, discarding both spellings of absent + # (null, and the zero sentinel), and fall back only if it never recorded a + # beginning. Deliberately `first` over the preference-ordered list and not + # `max` of it: max mixes "when it started" with "when it ended", which are + # different quantities, so it is not an ordering on runs at all. A run + # cancelled by the concurrency group does not stop the instant its + # replacement starts — the runner has to receive the signal and wind down — + # so predecessor.completedAt > successor.startedAt is the ordinary case + # (13s on the box#137 tip), and under max the dying predecessor + # out-dated its live replacement for the whole drain window. + # + # 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: ([.startedAt, .createdAt, .completedAt] | map(select(type == "string" and . != "" and (startswith("0001-01-01") | not))) - | max // ""), + | first // ""), outcome: ((.conclusion // .state // "") | ascii_upcase) } ] | group_by(.ctx) | map(sort_by([(.at == ""), .at]) | last | .outcome) as $latest diff --git a/CHANGELOG.md b/CHANGELOG.md index e290b1d..2075c5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,17 +51,28 @@ on the way to cutting its first release, and this file starts there. `SUCCESS` `scope` that superseded it, and judging every entry would have stranded every re-run PR in `needs-rebase`. - Which entry is newest is dated by the newest timestamp a run actually - carries, discarding *both* spellings of absent. A run still in flight has no - completion, but `gh` does not omit the field — its Go struct marshals the - zero time as the string `0001-01-01T00:00:00Z`, and `//` falls through - `null` and `false` only. Dating by completion therefore sorted the *live* - re-run to the bottom and let `last` pick the very run it superseded: a green - context with a replacement mid-flight read `SUCCESS`, inviting a merge the - button had already disabled — #136 restored by the fix for it. An entry - carrying no usable timestamp sorts last rather than first, so something that - cannot be dated is never discarded in favour of a stale success. Every - ambiguity here resolves toward "not settled". + Which entry is newest is dated by **when the run began** — `startedAt`, + falling back only if it never recorded one — discarding *both* spellings of + absent. A run still in flight has no completion, but `gh` does not omit the + field: its Go struct marshals the zero time as the string + `0001-01-01T00:00:00Z`, and `//` falls through `null` and `false` only. + Dating by completion therefore sorted the *live* re-run to the bottom and + let `last` pick the very run it superseded — a green context with a + replacement mid-flight read `SUCCESS`, inviting a merge the button had + already disabled, which is #136 restored by the fix for it. + + Taking the *newest* stamp each run carries is not a fix either, and this is + the subtle part: it compares a finished predecessor by when it **ended** + against a live successor by when it **began**, which is not an ordering on + runs at all. A run cancelled by the concurrency group does not stop the + instant its replacement starts — the runner has to receive the signal and + wind down — so the predecessor completing *after* the successor started is + the ordinary case, 13s wide on the box#137 tip that motivated the supersede + rule. For that whole drain window the dying predecessor out-dated its own + replacement. One consistent quantity, start time, is the only ordering that + holds. An entry carrying no usable timestamp at all sorts last rather than + first, so something that cannot be dated 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 @@ -74,10 +85,11 @@ on the way to cutting its first release, and this file starts there. is intent, so the reconciler never sets it — it only **clears** it once the PR stops being mergeable-by-a-human, which is precisely the staleness that made `needs-human` untrustworthy. Both live shapes, the mixed round, the - whole check-outcome enum, and the superseded and still-in-flight re-run — - in both spellings of an absent completion — are pinned in - `test/labels-reconcile.sh`. Ported from heavy-duty/box#137 so the three - repos' reconcilers stay byte-identical; fixtures 19 → 48. + whole check-outcome enum, and the re-run in every temporal arrangement it + occurs in — superseded, still in flight in both spellings of an absent + completion, undateable, and still draining past its replacement's start — + are pinned in `test/labels-reconcile.sh`. Ported from heavy-duty/box#137 so + the three repos' reconcilers stay byte-identical; fixtures 19 → 51. ### Added diff --git a/test/labels-reconcile.sh b/test/labels-reconcile.sh index 86be743..eea4561 100644 --- a/test/labels-reconcile.sh +++ b/test/labels-reconcile.sh @@ -277,11 +277,41 @@ expect "a replacement in flight for a CANCELLED run is pending, not failed" PEND "$(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 +# ambiguity resolves toward "not settled" rather than toward a stale success. +# Guarded by the sort tiebreak rather than the dating expression: reverting +# only `at:` leaves this passing, so the two changes are separately pinned. 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)" +# ...and the reverse direction, which stops "in flight sorts last" being +# widened into "in flight always wins": a run that FINISHED after an earlier +# in-flight entry is the newer word, and the context is settled. +expect "a finished re-run supersedes an earlier in-flight run" SUCCESS \ + "$(rollup "[$(inflight_ build 2026-07-20T15:19:00Z),\ + $(run_ build SUCCESS 2026-07-20T15:19:45Z)]" | checks_state)" + +# -- the DRAIN WINDOW. A run cancelled by the concurrency group does not stop +# the instant its replacement starts: the runner has to receive the signal +# and wind down, so the predecessor's completion lands AFTER the successor's +# start. On box#137's own tip that window was 13s (superseding run started +# 15:19:38, the run it cancelled finished 15:19:51). `run_()` cannot express +# it either — it carries no startedAt — so every fixture above spaces the +# predecessor's completion safely before the successor's start, and the whole +# window is invisible to them. This is why the run is dated by `first` of the +# preference-ordered stamps and not by `max` of them: max compares "when it +# ended" against "when it began", which is not an ordering on runs, and the +# dying predecessor out-dated its live replacement for the entire window. +drained_() { jq -n --arg n "$1" --arg c "$2" --arg s "$3" --arg e "$4" \ + '{__typename:"CheckRun", workflowName:"ci", name:$n, conclusion:$c, + startedAt:$s, completedAt:$e}'; } + +expect "a predecessor still draining does not out-date its live replacement" PENDING \ + "$(rollup "[$(drained_ build CANCELLED 2026-07-20T15:19:29Z 2026-07-20T15:19:51Z),\ + $(inflight_ build 2026-07-20T15:19:38Z)]" | checks_state)" +expect "...and the same when the draining predecessor is green (the #136 shape)" PENDING \ + "$(rollup "[$(drained_ build SUCCESS 2026-07-20T15:19:29Z 2026-07-20T15:19:51Z),\ + $(inflight_ build 2026-07-20T15:19:38Z)]" | 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.