diff --git a/.github/scripts/labels-reconcile.sh b/.github/scripts/labels-reconcile.sh index 048f62a..db00ea3 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 falling back only if it never recorded + # a beginning. NOT by the newest stamp of any kind: `max` compares the + # completion of a finished run against the start of a live one, which are + # different quantities and not an ordering on runs. A run cancelled by the + # concurrency group does not stop the instant its replacement starts — the + # runner has to wind down — so predecessor.completedAt > successor.startedAt + # is the ordinary case, and `max` dated the dead predecessor newer than the + # live run that replaced it, narrowing both failures above without closing + # them. The list is already in preference order, so `first` IS that rule. + # + # 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 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 f2654c5..0679af6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -103,16 +103,25 @@ 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`. 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". + stranded every re-run PR in `needs-rebase`. + + A run is dated by **when it began**, which took two corrections to get right + and both restored #136 in the meantime. Dating on completion fails 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 — so the live re-run sorted to the bottom and the run it superseded + was judged instead. Taking the *newest* stamp a run carries fails for a + subtler reason: it resolves to `completedAt` for a finished run and + `startedAt` for a live one, which are different quantities, so it never + ordered runs at all. A run cancelled by the concurrency group drains *after* + its replacement starts — 13 seconds on this PR's own `aa5a6ba` — so the dead + predecessor routinely out-dated the live run replacing it, and a green + predecessor in that window read `SUCCESS` with a re-run still in flight. + Start time has neither failure: a replacement always begins after the run it + replaces, whatever order they finish in. An entry carrying no usable stamp + sorts last rather than first, so an undateable in-flight run is never + discarded in favour of a stale success — every ambiguity 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 @@ -128,8 +137,9 @@ which records not just what changed but what each drill run proved. shapes, the mixed round, the whole check-outcome enum, and the in-flight re-run superseding both a green and a cancelled predecessor — in both directions, since a run that *finished* after an earlier in-flight entry - settles the context — are pinned in `test/labels-reconcile.sh` - (19 fixtures → 49). + settles the context, and across the drain window where the predecessor + completes last — are pinned in `test/labels-reconcile.sh` + (19 fixtures → 51). - **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 7a532ed..b68de99 100644 --- a/test/labels-reconcile.sh +++ b/test/labels-reconcile.sh @@ -291,6 +291,25 @@ 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 wind-down window. A predecessor cancelled by the concurrency group +# does not stop the instant its replacement starts, so its completion +# routinely lands AFTER the successor's start — on box's aa5a6ba the +# replacement started 15:19:38 and the run it cancelled finished 15:19:51. +# Dating by "newest stamp of any kind" compares the dead run's completion +# against the live run's start, which is not an ordering on runs, and the +# predecessor wins. Every fixture above spaces completion before start, so +# none of them can see it. run_() cannot express the overlap either — it +# carries no startedAt — hence the explicit payloads. +overlap_() { jq -n --arg n "$1" --arg o "$2" --arg s "$3" --arg c "$4" \ + '{__typename:"CheckRun", workflowName:"ci", name:$n, conclusion:$o, + startedAt:$s, completedAt:$c}'; } +expect "a predecessor finishing after its replacement started is still older (CANCELLED)" PENDING \ + "$(rollup "[$(overlap_ scope CANCELLED 2026-07-20T15:19:00Z 2026-07-20T15:19:51Z),\ + $(inflight_ scope 2026-07-20T15:19:38Z)]" | checks_state)" +expect "...and the same when it finished green — mid-flight is not mergeable" PENDING \ + "$(rollup "[$(overlap_ build SUCCESS 2026-07-20T15:19:00Z 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. DRAFT=false HEAD_SHA=head1 REQUESTED="$HUMAN" REVIEWS_JSON="$ALL_APPROVE" MERGEABLE=MERGEABLE