fix(labels): date a check run by when it started, not by a zero completion

The supersede collapse added in the previous commit dated each run by
`.completedAt // .startedAt // .createdAt`. 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 jq's `//` falls
through null and false only. The sentinel was therefore taken as the sort
key, and it sorts before every real timestamp — so the LIVE re-run became
the oldest entry in its context, `last` discarded it, and the run it
superseded was judged instead.

That restored #136 through the fix for it: a green context with a
replacement mid-flight reported SUCCESS, so a PR read mergeable, green,
all bots approve — state:needs-human — while branch protection had the
merge button disabled. It also narrowed rather than removed the flap the
supersede rule exists to prevent: between "run A cancelled by the
concurrency group" and "run B finishes", the PR reported FAILURE and the
agent was sent to fix something that was not broken.

Runs are now dated by the newest timestamp they actually carry, with both
spellings of absent discarded (null, and the zero sentinel). Entries that
carry no usable timestamp sort 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".

Found independently by claude-bot-andresmgsl and codex-bot-andresmgsl.
The fixtures could not have caught it: `run_()` always emits a real
completedAt, so every supersede fixture was a race between two finished
runs, and the bug lived in the one shape the helper could not express.
New `inflight_()` helper covers it; fixtures 44 -> 48.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
dan-claude-bot 2026-07-20 16:18:50 +00:00
parent a0951eacf8
commit 800571fdfb
3 changed files with 64 additions and 7 deletions

View file

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

View file

@ -51,6 +51,18 @@ 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".
`UNKNOWN` mergeability is deliberately not treated as unmergeable: GitHub
reports it for about a minute after every merge while it recomputes, and
flapping every open PR through `needs-rebase` on each merge would be worse
@ -61,10 +73,11 @@ on the way to cutting its first release, and this file starts there.
*which* PR to merge first, and order matters when they conflict. Queue order
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, and the
whole check-outcome enum are pinned in `test/labels-reconcile.sh`. Ported from
heavy-duty/box#137 so the three repos' reconcilers stay byte-identical;
fixtures 19 → 44.
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.
### Added

View file

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