diff --git a/actions/labels-reconcile/labels-reconcile.sh b/actions/labels-reconcile/labels-reconcile.sh index cd573f6..003c92c 100755 --- a/actions/labels-reconcile/labels-reconcile.sh +++ b/actions/labels-reconcile/labels-reconcile.sh @@ -55,6 +55,12 @@ LABELS="" # retirement heals the board instead of stranding a label nothing recomputes. RETIRED=(state:needs-rebase) STALE_AFTER=$((48 * 3600)) +# How long the facts behind blocker:unrequested must have stood still before it +# is written (#236 D2). The operator's "more than 5 minutes", measured off the +# inputs' own timestamps rather than off sweep memory — this script is +# stateless per pass and stays that way. Overridable the way this file's other +# constants are, for a caller whose round cadence is slower or faster. +RECONCILE_UNREQUESTED_GRACE="${RECONCILE_UNREQUESTED_GRACE:-300}" # The workflow whose runs checks_state must never grade — its own (#208). # GITHUB_WORKFLOW is ambient in every Actions step and names the CALLER (the # consumer's PR-facing workflow, since consumers name the caller), so this @@ -277,6 +283,8 @@ set_required_bots() { # the PR author is recused by construction # MERGEABLE MERGEABLE | CONFLICTING | UNKNOWN (GitHub's own verdict) # CHECKS SUCCESS | FAILURE | PENDING | NONE (the check rollup) # LABELS newline-separated labels currently on the PR +# HEAD_COMMIT_AT the head commit's own date, ISO-8601; empty when unread +# NOW this sweep's epoch seconds (main sets it once per run) # --------------------------------------------------------------------------- requested() { grep -qxF "$1" <<<"$REQUESTED"; } @@ -429,6 +437,44 @@ bot_verdict() { # $1 = login → MISSING | BLOCK | APPROVE | STALE | FEEDBACK esac } +iso_epoch() { # $1 = ISO-8601 timestamp → epoch seconds; nothing, rc 1, when unreadable + # An absent field reaches this as the empty string or as jq's literal "null"; + # both are "we did not read a time", and neither may be graded as one. + local at="${1-}" epoch + case "$at" in "" | null) return 1 ;; esac + epoch="$(date -d "$at" +%s 2>/dev/null)" || return 1 + [ -n "$epoch" ] || return 1 + printf '%s\n' "$epoch" +} + +unrequested_quiescent() { # 0 when the unrequested facts have stood for the grace (#236 D2) + # The stall blocker's supporting facts are the head and the round's newest + # submitted review: the ask it demands is owed only once both have stopped + # moving. Measured off those timestamps, not off sweep memory — ceremony#235 + # was flagged inside the ~90 seconds between a round-answer push and the + # author's re-request, because a sweep read the facts before the request + # landed and wrote after it. That is a round in motion, not a dropped ball. + # + # "Newest submitted review" is any submitted review, COMMENTED included: a + # non-verdict is still evidence the round is live, and counting it can only + # delay a flag, never invent one. + # + # A timestamp we could not read refuses the blocker (the standing rule: an + # unreadable fact never invents a verdict). This direction is deliberate and + # asymmetric — a missed flag costs one sweep of the 15-minute cadence, a + # false one flags a builder for doing exactly what BUILDER.md requires. + local newest verdict_at verdict_epoch + newest="$(iso_epoch "${HEAD_COMMIT_AT:-}")" || return 1 + verdict_at="$(jq -r '[.[].submitted_at] | max // empty' <<<"${REVIEWS_JSON:-[]}")" + if [ -n "$verdict_at" ]; then + # A round WITH verdicts whose newest one cannot be dated is unreadable, not + # quiescent; a round with no verdicts at all is simply the head's clock. + verdict_epoch="$(iso_epoch "$verdict_at")" || return 1 + [ "$verdict_epoch" -gt "$newest" ] && newest="$verdict_epoch" + fi + [ $((${NOW:-0} - newest)) -ge "$RECONCILE_UNREQUESTED_GRACE" ] +} + human_request_needed() { # 0 when needs-human requires a FRESH human request # already requested → the handoff is live; head-current human approval → # nothing left to ask. Anything else (never reviewed, an old comment, an @@ -464,7 +510,27 @@ blockers() { # → the blocker:* labels this PR should carry, one per line # A draft is exempt (the bots ignore drafts by design), and so is an # explicit human request — a maintainer claiming a PR early is deliberate, # not a dropped ball. - if [ "$DRAFT" != true ] && ! requested "$HUMAN"; then + # + # And so is a head whose checks have not answered yet (#236 D1). This is the + # one blocker that names an act the author must PERFORM, so it is the one + # that has to know when performing it is permitted: BUILDER.md's review round + # requires a green check at the head before requesting, so a builder waiting + # out a pending run is complying, and flagging compliance teaches its readers + # to ignore the label. Both 2026-08-03 instances were exactly that — + # crew#318 at ~12:44Z carried state:addressing + blocker:unrequested while + # the head's run was IN_PROGRESS, and ceremony#235 at 12:30Z caught the + # ~90-second gap between a round-answer push and the re-request. + # + # PENDING and FAILURE each already have an owner, which is why gating loses + # no coverage: on PENDING the next move is CI's and state:addressing / + # state:bots-reviewing already say what the PR is doing; on FAILURE + # blocker:ci-red owns that head, and stacking a second blocker on it + # double-flags one stall. NONE joins SUCCESS because no checks configured is + # nothing to wait for — the same reading the request rule gives the builder. + # UNREADABLE never arrives here: the caller skips the PR before deciding. + local checks_permit_the_ask=false + case "${CHECKS:-NONE}" in SUCCESS | NONE) checks_permit_the_ask=true ;; esac + if [ "$DRAFT" != true ] && [ "$checks_permit_the_ask" = true ] && ! requested "$HUMAN"; then local b v owed=false any_requested=false for b in "${REQUIRED_BOTS[@]}"; do requested "$b" && any_requested=true @@ -475,7 +541,10 @@ blockers() { # → the blocker:* labels this PR should carry, one per line v="$(bot_verdict "$b")" case "$v" in MISSING | STALE) owed=true ;; esac done - if [ "$owed" = true ] && [ "$any_requested" = false ]; then + # The quiescence grace (#236 D2) is the last question, after the debt is + # established: it asks whether the debt has stood long enough to be a + # dropped ball rather than a round still in motion. + if [ "$owed" = true ] && [ "$any_requested" = false ] && unrequested_quiescent; then echo blocker:unrequested fi fi @@ -933,6 +1002,29 @@ main() { log "#$n: read failed: $(read_failure_reason "$GH_VIEW_ERR")" exit 0 fi + # The head's own clock, for the blocker:unrequested grace (#236 D2). One + # read, pinned to the head SHA — not `gh pr view --json commits`, which + # asks for the FIRST hundred commits and would date a longer PR by a + # commit that is not its head. Last of the fetches on purpose: a PR the + # skip above walked away from must not pay for it, and neither do drafts, + # which never reach that blocker. Empty (a failed read, or a body without + # the field) leaves the blocker unjudged, by unrequested_quiescent. + HEAD_COMMIT_AT="" + if [ "$DRAFT" != true ]; then + HEAD_COMMIT_ERR_FILE="$(mktemp)" + HEAD_COMMIT_AT="$(gh api "repos/$REPO/commits/$HEAD_SHA" \ + --jq '.commit.committer.date' 2>"$HEAD_COMMIT_ERR_FILE" || echo "")" + HEAD_COMMIT_ERR="$(cat "$HEAD_COMMIT_ERR_FILE")" + rm -f "$HEAD_COMMIT_ERR_FILE" + case "$HEAD_COMMIT_AT" in + "" | null) + # Say why it degraded (#101 D2/D4), on its own line: this one + # narrows a blocker rather than skipping the PR, so it must not + # read as the wholly-blind shape the counted line above matches. + HEAD_COMMIT_AT="" + log "#$n: could not read the head commit's date: $(read_failure_reason "$HEAD_COMMIT_ERR") — blocker:unrequested not judged this pass" ;; + esac + fi reconcile_pr "$n" ) 2>&1 )" || status=$? diff --git a/changelog.d/236.md b/changelog.d/236.md new file mode 100644 index 0000000..2d76876 --- /dev/null +++ b/changelog.d/236.md @@ -0,0 +1,10 @@ +### Fixed + +- `blocker:unrequested` no longer fires while a head's checks are pending or + red: the review round forbids requesting there, so the one blocker that + demanded an act flagged builders for complying. Pending is CI's move, red is + `blocker:ci-red`'s (#236). +- `blocker:unrequested` now waits for the round to settle — the head and the + newest verdict must have stood for `RECONCILE_UNREQUESTED_GRACE` (default + 300s) — so a sweep landing between a push and its re-request no longer flags + a round in motion (#236). diff --git a/test/labels-reconcile.test.sh b/test/labels-reconcile.test.sh index f1b0aa6..c9ce210 100755 --- a/test/labels-reconcile.test.sh +++ b/test/labels-reconcile.test.sh @@ -38,6 +38,22 @@ rev() { # $1=login $2=state $3=commit $4=body $5=submitted_at → one review obj reviews() { jq -s '.' <<<"$*"; } # collect review objects into an array +# The blocker:unrequested quiescence inputs (#236 D2). Every fixture below +# inherits a readable, settled world — a head commit an hour before this +# sweep's clock — so the cases written before #236 assert exactly what they +# always asserted. The #236 block sets both per case. +# +# One consequence a new fixture has to know: a case that means to raise +# blocker:unrequested needs a REAL submitted_at on its reviews, because the +# grace dates the round's newest review. The symbolic stamps this file uses +# elsewhere (`t1`, `t2`, …) are not unreadable — GNU date reads `t1` as 01:00 +# in military timezone T, i.e. a time on WHATEVER day the suite runs — which is +# worse: the verdict would flip with the calendar, the hazard the LC_ALL pin at +# the top of this file guards on the other axis. Hence a fixed NOW here and +# real timestamps on the three stall fixtures below. +NOW="$(date -d 2026-08-03T12:00:00Z +%s)" +HEAD_COMMIT_AT=2026-08-03T11:00:00Z + # -- a sweep-wide read failure is visible without changing any PR ------------ warning="$(blind_sweep_warning 3 3 "HTTP 403: Resource not accessible by integration")" expect "a wholly blind sweep warns, leading with the observed reason" \ @@ -131,8 +147,8 @@ expect "requested bots mean bots-reviewing" state:bots-reviewing "$(decide_state # With a live request that is the bots' ball; with NO request outstanding it # is the agent's, because nothing is coming until somebody asks. REQUESTED="$BOT3" REVIEWS_JSON="$(reviews \ - "$(rev "$BOT1" APPROVED head1 "" t1)" \ - "$(rev "$BOT2" APPROVED head1 "" t2)")" + "$(rev "$BOT1" APPROVED head1 "" 2026-08-03T10:00:00Z)" \ + "$(rev "$BOT2" APPROVED head1 "" 2026-08-03T10:01:00Z)")" expect "a missing bot WITH a live request is bots-reviewing" state:bots-reviewing "$(decide_state)" REQUESTED="" expect "...but with nobody asked it is the agent's ball" state:addressing "$(decide_state)" @@ -283,15 +299,15 @@ expect "...and raises no blocker" "" "$(blockers)" MERGEABLE=MERGEABLE CHECKS=SUCCESS REQUESTED="" REVIEWS_JSON='[]' expect "ready, nobody asked, nothing reviewed raises unrequested" blocker:unrequested "$(blockers)" # ...the partial case is equally stalled: one verdict in, nobody asked for the rest -REVIEWS_JSON="$(reviews "$(rev "$BOT1" APPROVED head1 "" t1)")" +REVIEWS_JSON="$(reviews "$(rev "$BOT1" APPROVED head1 "" 2026-08-03T10:00:00Z)")" expect "one bot in, none requested is still unrequested" blocker:unrequested "$(blockers)" # ...a STALE round with nobody asked is the same debt, and arguably worse: the # page carries approvals that no longer describe the tree. Guarding on # MISSING alone let this one through with no blocker at all. REVIEWS_JSON="$(reviews \ - "$(rev "$BOT1" APPROVED oldhead "" t1)" \ - "$(rev "$BOT2" APPROVED oldhead "" t2)" \ - "$(rev "$BOT3" APPROVED oldhead "" t3)")" + "$(rev "$BOT1" APPROVED oldhead "" 2026-08-03T10:00:00Z)" \ + "$(rev "$BOT2" APPROVED oldhead "" 2026-08-03T10:01:00Z)" \ + "$(rev "$BOT3" APPROVED oldhead "" 2026-08-03T10:02:00Z)")" expect "a stale round with nobody asked is unrequested too" blocker:unrequested "$(blockers)" expect "...and is still the agent's ball" state:addressing "$(decide_state)" # ...but a live request means an answer IS coming @@ -824,6 +840,77 @@ expect "each blind PR logs its reason as its own line beside the counted one" 2 expect "exactly the blind PRs match the counted shape whole-line — no more, no less" 2 \ "$(grep -c '^labels: #[0-9]*: could not read mergeability/checks — left alone this pass$' <<<"$blind_main")" +# -- the grace's own wiring: the fixtures above prove the predicate, and only a +# sweep can prove the fetch that feeds it (#236 D2). The fixture-only version +# of this change would have passed with the global never assigned — the #91 +# shape, where the probes could not reach the per-PR path at all. +unrequested_main_probe() { # $1 = read | denied, the head-commit read's outcome + ( + GITHUB_EVENT_NAME=schedule + REPO=owner/repo + LABELS_CONF=.github/labels.conf + UMODE="$1" + gh() { + if [ "$1" = label ] && [ "$2" = list ]; then core_label_rows | cut -d'|' -f1; return 0; fi + if [ "$1" = pr ] && [ "$2" = list ]; then printf '303\n'; return 0; fi + if [ "$1" = pr ] && [ "$2" = view ]; then + # green, so the D1 gate is open and D2 is the only question left + jq -n '{mergeable:"MERGEABLE", + statusCheckRollup:[{__typename:"CheckRun",workflowName:"ci", + name:"check",conclusion:"SUCCESS", + startedAt:"2026-07-01T00:00:00Z"}]}' + return 0 + fi + # recorded to a file, not to stdout: reconcile_pr sends the edit call's + # stdout to /dev/null, so a narrating stub would look like no edit at all + if [ "$1" = issue ] && [ "$2" = edit ]; then printf '%s\n' "$*" >>"$RTMP/uedits-$UMODE"; return 0; fi + [ "$1" = api ] || return 0 + shift + local jqexpr="" endpoint="" + while [ $# -gt 0 ]; do + case "$1" in + --jq) jqexpr="$2"; shift ;; + -*) ;; + *) [ -n "$endpoint" ] || endpoint="$1" ;; + esac + shift + done + case "$endpoint" in + */commits/*) # the head-commit read; ordered before the commit LIST below + if [ "$UMODE" = denied ]; then + printf 'gh: Not Found (HTTP 404)\n' >&2 + return 1 + fi + jq -n '{commit:{committer:{date:"2026-07-01T00:00:00Z"}}}' | jq -r "${jqexpr:-.}" ;; + */pulls/303) + jq -n '{draft:false,user:{login:"author"},head:{sha:"headsha"}, + base:{sha:"basesha"},labels:[],requested_reviewers:[], + created_at:"2026-07-01T00:00:00Z"}' ;; + *) printf '[]\n' | jq -r "${jqexpr:-.}" ;; # every collection empty + esac + } + main + ) +} + +read_sweep="$(unrequested_main_probe read)" +expect "the sweep reads the head's date and writes the stall it now dates" yes \ + "$(grep -q 'blocker:unrequested' "$RTMP/uedits-read" && echo yes || echo no)" +expect "...saying nothing about a degraded read" no \ + "$(grep -q "could not read the head commit's date" <<<"$read_sweep" && echo yes || echo no)" +denied_sweep="$(unrequested_main_probe denied)" +expect "a denied head-commit read names the denial (#101's shape)" yes \ + "$(grep -q "^labels: #303: could not read the head commit's date: gh: Not Found (HTTP 404)" <<<"$denied_sweep" \ + && echo yes || echo no)" +expect "...and writes no blocker it could not date" no \ + "$(grep -q 'blocker:unrequested' "$RTMP/uedits-denied" && echo yes || echo no)" +# ...while the PR is still converged: this read narrows one blocker, it does not +# skip the PR the way an unreadable rollup does +expect "...while the state still converges — one blocker unjudged, not a skip" yes \ + "$(grep -q 'state:addressing' "$RTMP/uedits-denied" && echo yes || echo no)" +expect "...and the sweep does not report it as a blind pass" 0 \ + "$(grep -c 'could not read mergeability/checks' <<<"$denied_sweep" || true)" + # --------------------------------------------------------------------------- # bootstrap_labels retires the GitHub defaults (#93). LABELS.md published # them as deleted at bootstrap; nothing deleted them — incubator's first @@ -1057,6 +1144,128 @@ REQUESTED="" REVIEWS_JSON='[]' expect "a draft with no round history still reads building" \ state:building "$(decide_state)" +# --------------------------------------------------------------------------- +# blocker:unrequested knows when the ask is permitted (#236). The blocker +# demands an act — request the panel — that BUILDER.md forbids under a head +# whose checks have not answered, so the predicate that flags the omission has +# to read CHECKS and has to let a round in motion finish moving. Two guards, +# each proved load-bearing by a mutation at the end of the block. +# --------------------------------------------------------------------------- +DRAFT=false HEAD_SHA=head1 REQUESTED="" MERGEABLE=MERGEABLE LABELS="" +NOW="$(date -d 2026-08-03T12:00:00Z +%s)" +# the genuine #26/#39 debt: three approvals of a head a push staled, nobody +# asked for the re-verdicts, and every fact hours old +OWED_QUIET_ROUND="$(reviews \ + "$(rev "$BOT1" APPROVED oldhead "" 2026-08-03T10:00:00Z)" \ + "$(rev "$BOT2" APPROVED oldhead "" 2026-08-03T10:01:00Z)" \ + "$(rev "$BOT3" APPROVED oldhead "" 2026-08-03T10:02:00Z)")" +REVIEWS_JSON="$OWED_QUIET_ROUND" HEAD_COMMIT_AT=2026-08-03T11:00:00Z +CHECKS=SUCCESS +expect "green, quiescent, owed and unasked is the stall (the control)" \ + blocker:unrequested "$(blockers)" +# D1 — the gate. crew#318's shape: the same debt under a running check, where +# requesting is the one thing the builder must not do. +CHECKS=PENDING +expect "a pending head is CI's move, not a dropped ask" "" "$(blockers)" +CHECKS=FAILURE +expect "a red head raises ci-red alone — the two never co-occur" \ + blocker:ci-red "$(blockers)" +CHECKS=NONE +expect "no checks configured is nothing to wait for, so the stall still shows" \ + blocker:unrequested "$(blockers)" +# D2 — the grace. ceremony#235's shape: a sweep landing in the ~90 seconds +# between a round-answer push and the author's re-request. +CHECKS=SUCCESS HEAD_COMMIT_AT=2026-08-03T11:57:30Z +expect "a head pushed inside the grace is a round in motion, not a stall" \ + "" "$(blockers)" +HEAD_COMMIT_AT=2026-08-03T11:55:00Z +expect "...and exactly at the grace it flags — the boundary is inclusive" \ + blocker:unrequested "$(blockers)" +HEAD_COMMIT_AT=2026-08-03T11:50:00Z +expect "...and a later pass flags it with nothing else changed" \ + blocker:unrequested "$(blockers)" +# a verdict is the other supporting fact, and an old head does not license +# flagging a round whose newest verdict landed a minute ago +HEAD_COMMIT_AT=2026-08-03T10:00:00Z +REVIEWS_JSON="$(reviews "$(rev "$BOT1" APPROVED oldhead "" 2026-08-03T11:59:00Z)")" +expect "a verdict submitted inside the grace is motion too" "" "$(blockers)" +# no verdicts at all is not an unreadable round — it is the first-ask stall, +# and the head's clock is the whole of it +REVIEWS_JSON='[]' HEAD_COMMIT_AT=2026-08-03T11:00:00Z +expect "nothing reviewed and nobody asked flags off the head's clock alone" \ + blocker:unrequested "$(blockers)" +# an unreadable fact never invents a verdict — the standing rule, applied to +# both timestamps +REVIEWS_JSON="$OWED_QUIET_ROUND" HEAD_COMMIT_AT="" +expect "an unread head date leaves the blocker unjudged" "" "$(blockers)" +HEAD_COMMIT_AT=null +expect "...and jq's literal null is unread, not epoch zero" "" "$(blockers)" +HEAD_COMMIT_AT=2026-08-03T11:00:00Z +REVIEWS_JSON="$(reviews "$(rev "$BOT1" APPROVED oldhead "" not-a-timestamp)")" +expect "a round whose newest verdict cannot be dated is unread, not quiescent" \ + "" "$(blockers)" +# the constant is overridable the way this file's others are +REVIEWS_JSON="$OWED_QUIET_ROUND" HEAD_COMMIT_AT=2026-08-03T11:57:30Z +RECONCILE_UNREQUESTED_GRACE=60 +expect "a shorter configured grace flags the same facts" \ + blocker:unrequested "$(blockers)" +RECONCILE_UNREQUESTED_GRACE=300 + +# the timestamp reader, directly: the three unreadable spellings it must refuse +expect "iso_epoch reads a real stamp" \ + "$(date -d 2026-08-03T12:00:00Z +%s)" "$(iso_epoch 2026-08-03T12:00:00Z)" +expect "iso_epoch refuses an absent stamp" "" "$(iso_epoch "")" +expect "iso_epoch refuses jq's null" "" "$(iso_epoch null)" +expect "iso_epoch refuses a stamp date cannot read" "" "$(iso_epoch not-a-timestamp)" +# ...and the trap it does NOT catch, recorded because a fixture author will +# reach for it: `t1` is a valid date to GNU date — 01:00 in military timezone T, +# on the day the suite runs — so it reads as a moving stamp rather than as an +# unreadable one. Real timestamps in any fixture the grace touches. +expect "a symbolic stamp is readable, and moves with the run's day" \ + "$(date -d t1 +%s)" "$(iso_epoch t1)" + +# -- the mutation proofs: both guards are load-bearing, and this runs them ---- +# A guard the fixtures cannot see removed is a guard nobody is testing, so each +# is deleted from a COPY of the script and the fixture that covers it must flip. +# The sed programs target one token each, so a refactor that moves a guard +# fails here loudly instead of passing silently. +mutant_blockers() { # $1 = sed program → blockers() from a copy of the script + # The copy keeps its position in the tree — the script sources lib/ruling.sh + # relative to its own path, and a copy dropped anywhere else would source + # nothing and say so on stderr instead of failing. + local root="$RTMP/mutant" mutated + mutated="$root/actions/labels-reconcile/labels-reconcile.sh" + mkdir -p "$root/actions/labels-reconcile" + ln -sfn "$PWD/lib" "$root/lib" + sed "$1" actions/labels-reconcile/labels-reconcile.sh >"$mutated" + DRAFT="$DRAFT" HEAD_SHA="$HEAD_SHA" REQUESTED="$REQUESTED" \ + REVIEWS_JSON="$REVIEWS_JSON" MERGEABLE="$MERGEABLE" CHECKS="$CHECKS" \ + NOW="$NOW" HEAD_COMMIT_AT="$HEAD_COMMIT_AT" \ + RECONCILE_UNREQUESTED_GRACE="$RECONCILE_UNREQUESTED_GRACE" \ + bash -u -c ' + . "$1" + load_config .github/labels.conf + set_required_bots codex-bot-andresmgsl + blockers + ' bash "$mutated" +} +# the harness itself, unmutated: it must reproduce the verdict the sourced +# functions give, or a "flip" below proves nothing about the guard +REVIEWS_JSON="$OWED_QUIET_ROUND" HEAD_COMMIT_AT=2026-08-03T11:00:00Z CHECKS=SUCCESS +expect "the mutation harness reproduces the control verdict" \ + blocker:unrequested "$(mutant_blockers 's/^#no-such-line$//')" +CHECKS=PENDING +expect "...and the pending fixture is green in the unmutated copy" \ + "" "$(mutant_blockers 's/^#no-such-line$//')" +expect "removing the green gate reds the pending fixture" \ + blocker:unrequested \ + "$(mutant_blockers 's/checks_permit_the_ask=false/checks_permit_the_ask=true/')" +CHECKS=SUCCESS HEAD_COMMIT_AT=2026-08-03T11:57:30Z +expect "removing the grace reds the inside-the-window fixture" \ + blocker:unrequested \ + "$(mutant_blockers 's/ \&\& unrequested_quiescent//')" +HEAD_COMMIT_AT=2026-08-03T11:00:00Z + # -- per-author panels (#224): the required set flows from the one ---------- # resolution point, and convergence counts the effective set — never the # base panel beside a reduced request set (the must-fail the issue names)