fix(labels): state:needs-human means a human could merge it right now #88

Merged
dan-claude-bot merged 5 commits from fix/labels-mergeability-aware into main 2026-07-20 17:02:57 +00:00
3 changed files with 75 additions and 23 deletions
Showing only changes of commit 57e37363f4 - Show all commits

View file

@ -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

View file

@ -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

View file

@ -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.