fix(labels): date a check run by when it BEGAN, not by the newest stamp it carries
Third and final correction to the supersede rule, and the second one
that silently restored #136.
Round 3 replaced "date by completion" with "date by the newest stamp
the run carries". That is still not an ordering on runs: `max` over
[startedAt, createdAt, completedAt] resolves to completedAt for a
FINISHED run and startedAt for a LIVE one — different quantities, so
the comparison was never between like and like.
The consequence is the ordinary concurrency-group path, not an edge
case. A run cancelled by a concurrency group does not stop instantly;
it drains AFTER its replacement has already started, so
predecessor.completedAt > successor.startedAt is the normal shape. On
this PR's own aa5a6ba the window was 13 seconds. Inside it the dead
predecessor out-dated the live run replacing it, and a green
predecessor reported SUCCESS while a re-run was still in flight:
SUCCESS completing 15:30:13, replacement started 15:30:00
max -> SUCCESS (#136: needs-human over a disabled merge button)
first -> PENDING
Fixed with `max` -> `first`. The list is already in preference order,
so `first` IS "date it by when it began" — and a replacement always
begins after the run it replaces, whatever order they finish in. The
sentinel filtering and the undateable-sorts-last tiebreak are
unchanged; this narrows the rule to a quantity that actually orders.
Prescribed independently by claude-bot-andresmgsl and
codex-bot-andresmgsl.
Two fixtures pin the drain window in both colours. 49 -> 51.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
cbc8f4f7a8
commit
b9527d3cc1
3 changed files with 58 additions and 19 deletions
24
.github/scripts/labels-reconcile.sh
vendored
24
.github/scripts/labels-reconcile.sh
vendored
|
|
@ -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
|
||||
|
|
|
|||
34
CHANGELOG.md
34
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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in a new issue